Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/23/2011 06:02 PM, Brad King wrote: > On 06/23/2011 10:36 AM, Michael Wild wrote: >> On 06/23/2011 04:11 PM, Brad King wrote: >>> We manually select topics from 'next' and merge them to 'master'. >>> Only topics in master will be released. >> >> So, how does that work? Do you look for the merges in 'next' that you >> like, and then re-merge them to 'master' directly from the topic-branch? > > Yes. The complete workflow is described generically here: > > http://public.kitware.com/Wiki/Git/Workflow/Topic > > We use a "topic stage" repository to keep explicit topic branch heads > that are not in all integration branches: > > http://public.kitware.com/Wiki/Git/Workflow/Stage > > CMake's topic stage is published here: > > http://cmake.org/stage/cmake.git > > The set of branches changes regularly as topics are added or finished. > >> Something like this? >> >> -- A I -- master >> \ \ \/ >> \ \ C --- D -- G -- topic2 >> \ \ \ >>\ B -- D -- topic1 \ >> \\ \ >> \--- F H -- next > > Yes, exactly. > >> i.e. everything starts at A, 'master' and 'next' being identical. Then, >> someone creates 'topic1' and merges it into 'next' (commit F). >> Meanwhile, another topic, 'topic2' is created, and also merged into >> 'next' (commit H). Now, 'topic1' just isn't ready, but 'topic2' is, so >> it gets merged into 'master' (commmit I). Is this correct? > > Yes. It gets a little more complicated when there are conflicts: > > http://public.kitware.com/Wiki/Git/Workflow/Topic/Conflicts > >> Is 'next' kind of your "throw-away" integration branch? > > Yes. No topics are allowed to see any of the merges into next. There > is even a check on the server that disallows topics (or master) to see > the beginning of next. > >> Do you rebase it regularly onto master? > > So far we haven't rewritten it but the workflow allows that because > nothing should be based on it. Currently we have to avoid rewriting > the branch because some dashboard clients may do just "git pull" to > update. Recent CTest versions do a fetch & reset so that they can > handle upstream rewrites. > > I designed all of the above after reading about Git's own workflow: > > > http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/howto/maintain-git.txt;hb=v1.7.5 > > Our "topic stage" takes the place of the maintainer's private repository. > I designed it to help distribute some of the maintainer's workload among > developers (and they don't even have to know; it's just part of their > workflow). > > -Brad Thanks for the thorough explanation. I have to say, it is very well thought-through, I'll try to enforce something similar for my projects. gitworkflows(7) always seemed a bit unwieldy to me, but then it is also geared towards much larger projects with many more contributors... Michael ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/23/2011 10:36 AM, Michael Wild wrote: > On 06/23/2011 04:11 PM, Brad King wrote: >> We manually select topics from 'next' and merge them to 'master'. >> Only topics in master will be released. > > So, how does that work? Do you look for the merges in 'next' that you > like, and then re-merge them to 'master' directly from the topic-branch? Yes. The complete workflow is described generically here: http://public.kitware.com/Wiki/Git/Workflow/Topic We use a "topic stage" repository to keep explicit topic branch heads that are not in all integration branches: http://public.kitware.com/Wiki/Git/Workflow/Stage CMake's topic stage is published here: http://cmake.org/stage/cmake.git The set of branches changes regularly as topics are added or finished. > Something like this? > > -- A I -- master > \ \ \/ > \ \ C --- D -- G -- topic2 > \ \ \ >\ B -- D -- topic1 \ > \\ \ > \--- F H -- next Yes, exactly. > i.e. everything starts at A, 'master' and 'next' being identical. Then, > someone creates 'topic1' and merges it into 'next' (commit F). > Meanwhile, another topic, 'topic2' is created, and also merged into > 'next' (commit H). Now, 'topic1' just isn't ready, but 'topic2' is, so > it gets merged into 'master' (commmit I). Is this correct? Yes. It gets a little more complicated when there are conflicts: http://public.kitware.com/Wiki/Git/Workflow/Topic/Conflicts > Is 'next' kind of your "throw-away" integration branch? Yes. No topics are allowed to see any of the merges into next. There is even a check on the server that disallows topics (or master) to see the beginning of next. > Do you rebase it regularly onto master? So far we haven't rewritten it but the workflow allows that because nothing should be based on it. Currently we have to avoid rewriting the branch because some dashboard clients may do just "git pull" to update. Recent CTest versions do a fetch & reset so that they can handle upstream rewrites. I designed all of the above after reading about Git's own workflow: http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/howto/maintain-git.txt;hb=v1.7.5 Our "topic stage" takes the place of the maintainer's private repository. I designed it to help distribute some of the maintainer's workload among developers (and they don't even have to know; it's just part of their workflow). -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/23/2011 04:11 PM, Brad King wrote: > On 06/23/2011 09:53 AM, Alexander Neundorf wrote: >> On Thursday 23 June 2011, Brad King wrote: >>> On 06/23/2011 05:12 AM, Alexander Neundorf wrote: > Please put it at the very bottom of the entire > documentation, just above the note about NO_POLICY_SCOPE. Done. >>> >>> Thanks. The topic looks good. Please merge it to 'next' when >>> you're ready. >> >> Won't this have the effect that it will be in 2.8.5 ? > > No. We manually select topics from 'next' and merge them to 'master'. > Only topics in master will be released. > > http://www.cmake.org/Wiki/CMake/Git#Branches > > -Brad So, how does that work? Do you look for the merges in 'next' that you like, and then re-merge them to 'master' directly from the topic-branch? Something like this? -- A I -- master \ \ \/ \ \ C --- D -- G -- topic2 \ \ \ \ B -- D -- topic1 \ \\ \ \--- F H -- next i.e. everything starts at A, 'master' and 'next' being identical. Then, someone creates 'topic1' and merges it into 'next' (commit F). Meanwhile, another topic, 'topic2' is created, and also merged into 'next' (commit H). Now, 'topic1' just isn't ready, but 'topic2' is, so it gets merged into 'master' (commmit I). Is this correct? Is 'next' kind of your "throw-away" integration branch? Do you rebase it regularly onto master? Michael ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/23/2011 09:53 AM, Alexander Neundorf wrote: > On Thursday 23 June 2011, Brad King wrote: >> On 06/23/2011 05:12 AM, Alexander Neundorf wrote: Please put it at the very bottom of the entire documentation, just above the note about NO_POLICY_SCOPE. >>> >>> Done. >> >> Thanks. The topic looks good. Please merge it to 'next' when >> you're ready. > > Won't this have the effect that it will be in 2.8.5 ? No. We manually select topics from 'next' and merge them to 'master'. Only topics in master will be released. http://www.cmake.org/Wiki/CMake/Git#Branches -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Thursday 23 June 2011, Brad King wrote: > On 06/23/2011 05:12 AM, Alexander Neundorf wrote: > >> Please put it at the very bottom of the entire > >> documentation, just above the note about NO_POLICY_SCOPE. > > > > Done. > > Thanks. The topic looks good. Please merge it to 'next' when > you're ready. Won't this have the effect that it will be in 2.8.5 ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/23/2011 05:12 AM, Alexander Neundorf wrote: >> Please put it at the very bottom of the entire >> documentation, just above the note about NO_POLICY_SCOPE. > > Done. Thanks. The topic looks good. Please merge it to 'next' when you're ready. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Wednesday 22 June 2011, Brad King wrote: > On 06/21/2011 03:25 PM, Alexander Neundorf wrote: > > Ok, I pushed the branch again. > > The new name looks better. Perhaps I missed this last time, but the > documentation of the variable within the command is back up in the > simple section. Yes, you did ;-) Actually I thought you were ok with it. > Please put it at the very bottom of the entire > documentation, just above the note about NO_POLICY_SCOPE. Done. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/21/2011 03:25 PM, Alexander Neundorf wrote: > Ok, I pushed the branch again. The new name looks better. Perhaps I missed this last time, but the documentation of the variable within the command is back up in the simple section. Please put it at the very bottom of the entire documentation, just above the note about NO_POLICY_SCOPE. Other than that, this looks good. Thanks, -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Tuesday 21 June 2011, Brad King wrote: > On 06/20/2011 06:23 PM, Alexander Neundorf wrote: > > I moved part of the documentation now to the variable section. > > > > Better ? > > Nice, thanks. > > > While looking at it, I'm not sure anymore the name > > "DISABLE_FIND_PACKAGE_" is good. > > > > Should it maybe have the "CMAKE_" prefix ? > > Yes, good idea. It is a variable that controls behavior of the > language so that prefix makes sense. > > > CMAKE_DISABLE_FIND_PACKAGE_ ? > > or > > CMAKE_FIND_PACKAGE_DISABLE_ ? > > Either one is fine with me. Ok, I pushed the branch again. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/20/2011 06:23 PM, Alexander Neundorf wrote: > I moved part of the documentation now to the variable section. > > Better ? Nice, thanks. > While looking at it, I'm not sure anymore the name > "DISABLE_FIND_PACKAGE_" is good. > > Should it maybe have the "CMAKE_" prefix ? Yes, good idea. It is a variable that controls behavior of the language so that prefix makes sense. > CMAKE_DISABLE_FIND_PACKAGE_ ? > or > CMAKE_FIND_PACKAGE_DISABLE_ ? Either one is fine with me. Thanks, -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Monday 20 June 2011, Brad King wrote: > On 06/20/2011 12:40 PM, Alexander Neundorf wrote: > > On Monday 20 June 2011, Brad King wrote: > >> - In the documentation patch please move mention of this variable down > >> > >> to the bottom. It is not important information for general authors > >> learning the command. Certainly this information does not belong > >> in the short usage summary section above the full command signature. > > > > Well, I put it at the end of the "simple" documentation and before the > > details, because IMO it is for users of the package and not for > > developers (which may need the details), so I thought it should be > > relatively early. > > > > But I don't have a strong opinion on that. > > Since it is a variable and not an argument to the command I suggest that > it be added to cmDocumentVariables too. Put the full detailed description > there and perhaps also describe the intended use case. Then just add a > once-sentence summary and reference to the variable at the end of the > find_package details documentation. That way you can add as much detail > as we need without cluttering the main command documentation. I moved part of the documentation now to the variable section. Better ? While looking at it, I'm not sure anymore the name "DISABLE_FIND_PACKAGE_" is good. Should it maybe have the "CMAKE_" prefix ? CMAKE_DISABLE_FIND_PACKAGE_ ? or CMAKE_FIND_PACKAGE_DISABLE_ ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Monday 20 June 2011, Alexander Neundorf wrote: > On Monday 20 June 2011, Brad King wrote: > ... > > > As you guessed, interactive rebase is the correct approach. Since > > you have not merged the topic you are free to overwrite it on the > > topic stage. The old version of the history will be replaced with > > the new version. It might feel funny the first time but it is nice > > and easy once you get the idea. I use it all the time locally to > > clean up and organize a topic into a logical series of changes > > before pushing it out. > > > > -Brad > > Hmm, I think I managed the rebasing, but now git complains when I try to > push: > > hammer:~/src/CMake/CMake-git$ git push stage HEAD Ok, git push --force stage HEAD seems to have worked Ok. Is there a better way then using --force ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/20/2011 05:14 PM, Alexander Neundorf wrote: > Hmm, I think I managed the rebasing, but now git complains when I try to push: > > hammer:~/src/CMake/CMake-git$ git push stage HEAD > To g...@public.kitware.com:stage/cmake.git > ! [rejected]HEAD -> DisableSwitchForFindPackage (non-fast-forward) > error: failed to push some refs to 'g...@public.kitware.com:stage/cmake.git' > To prevent you from losing history, non-fast-forward updates were rejected > Merge the remote changes (e.g. 'git pull') before pushing again. See the > 'Note about fast-forwards' section of 'git push --help' for details. Use "+" in your refspec: git push stage +HEAD to overwrite the existing branch. Use with care. > Why should I need to pull, nobody else pushed to this branch ? Those are just generic instructions Git gives when this type of push fails. The author of the instructions is assuming a certain workflow which we are not using. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Monday 20 June 2011, Brad King wrote: ... > As you guessed, interactive rebase is the correct approach. Since > you have not merged the topic you are free to overwrite it on the > topic stage. The old version of the history will be replaced with > the new version. It might feel funny the first time but it is nice > and easy once you get the idea. I use it all the time locally to > clean up and organize a topic into a logical series of changes > before pushing it out. > > -Brad Hmm, I think I managed the rebasing, but now git complains when I try to push: hammer:~/src/CMake/CMake-git$ git push stage HEAD To g...@public.kitware.com:stage/cmake.git ! [rejected]HEAD -> DisableSwitchForFindPackage (non-fast-forward) error: failed to push some refs to 'g...@public.kitware.com:stage/cmake.git' To prevent you from losing history, non-fast-forward updates were rejected Merge the remote changes (e.g. 'git pull') before pushing again. See the 'Note about fast-forwards' section of 'git push --help' for details. Why should I need to pull, nobody else pushed to this branch ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/20/2011 07:46 PM, Brad King wrote: > On 06/20/2011 12:40 PM, Alexander Neundorf wrote: > >> What is the recommended way how to do this with git ? >> >> Simply add one more commit which does that or do I have to do something with >> rebase --interactive ? >> How does that play together with the branch I already pushed ? Will this >> pushed branch get a new history ? > > As you guessed, interactive rebase is the correct approach. Since > you have not merged the topic you are free to overwrite it on the > topic stage. The old version of the history will be replaced with > the new version. It might feel funny the first time but it is nice > and easy once you get the idea. I use it all the time locally to > clean up and organize a topic into a logical series of changes > before pushing it out. > > -Brad For this kind of work, I also like to use StGit. It is similar to Quilt, but makes full use of Git. Michael ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/20/2011 12:40 PM, Alexander Neundorf wrote: > On Monday 20 June 2011, Brad King wrote: >> - In the documentation patch please move mention of this variable down >> to the bottom. It is not important information for general authors >> learning the command. Certainly this information does not belong >> in the short usage summary section above the full command signature. > > Well, I put it at the end of the "simple" documentation and before the > details, because IMO it is for users of the package and not for developers > (which may need the details), so I thought it should be relatively early. > > But I don't have a strong opinion on that. Since it is a variable and not an argument to the command I suggest that it be added to cmDocumentVariables too. Put the full detailed description there and perhaps also describe the intended use case. Then just add a once-sentence summary and reference to the variable at the end of the find_package details documentation. That way you can add as much detail as we need without cluttering the main command documentation. > What is the recommended way how to do this with git ? > > Simply add one more commit which does that or do I have to do something with > rebase --interactive ? > How does that play together with the branch I already pushed ? Will this > pushed branch get a new history ? As you guessed, interactive rebase is the correct approach. Since you have not merged the topic you are free to overwrite it on the topic stage. The old version of the history will be replaced with the new version. It might feel funny the first time but it is nice and easy once you get the idea. I use it all the time locally to clean up and organize a topic into a logical series of changes before pushing it out. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Monday 20 June 2011, Alexander Neundorf wrote: ... > What is the recommended way how to do this with git ? with "how to do this" I mean "how to make the suggested fixes" Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Monday 20 June 2011, Brad King wrote: > On 06/17/2011 12:55 PM, Alexander Neundorf wrote: > > On Thursday 16 June 2011, Brad King wrote: > >> On 06/16/2011 04:15 PM, Alexander Neundorf wrote: > >>> I'll push a branch to the stage once 2.8.5 is released. Or can I do > >>> that earlier ? > >> > >> You can push it any time but skip merging it. > > > > There should be a branch DisableSwitchForFindPackage now in stage. > > Please have a look at it :-) > > David and I just looked through this. It looks pretty good. > Here are some comments: > > - In the first patch "return true" is indented incorrectly > > - In the documentation patch please move mention of this variable down > to the bottom. It is not important information for general authors > learning the command. Certainly this information does not belong > in the short usage summary section above the full command signature. Well, I put it at the end of the "simple" documentation and before the details, because IMO it is for users of the package and not for developers (which may need the details), so I thought it should be relatively early. But I don't have a strong opinion on that. > - The related-cache-entry warning is unnecessary overhead and does quite > a bit of work for a command that is supposed to do nothing. The stated > use case of this feature is for packagers to add -DDISABLE_... arguments > to scripted calls to the "cmake" command line. If they are adding extra > unused arguments defining things related to a disabled package then the > generic cli warning infrastructure will already complain about them. I wasn't completely sure whether this warning is a good idea or not, that's why it is in a separate commit. What is the recommended way how to do this with git ? Simply add one more commit which does that or do I have to do something with rebase --interactive ? How does that play together with the branch I already pushed ? Will this pushed branch get a new history ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/17/2011 12:55 PM, Alexander Neundorf wrote: > On Thursday 16 June 2011, Brad King wrote: >> On 06/16/2011 04:15 PM, Alexander Neundorf wrote: >>> I'll push a branch to the stage once 2.8.5 is released. Or can I do that >>> earlier ? >> >> You can push it any time but skip merging it. > > There should be a branch DisableSwitchForFindPackage now in stage. > Please have a look at it :-) David and I just looked through this. It looks pretty good. Here are some comments: - In the first patch "return true" is indented incorrectly - In the documentation patch please move mention of this variable down to the bottom. It is not important information for general authors learning the command. Certainly this information does not belong in the short usage summary section above the full command signature. - The related-cache-entry warning is unnecessary overhead and does quite a bit of work for a command that is supposed to do nothing. The stated use case of this feature is for packagers to add -DDISABLE_... arguments to scripted calls to the "cmake" command line. If they are adding extra unused arguments defining things related to a disabled package then the generic cli warning infrastructure will already complain about them. Thanks, -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Thursday 16 June 2011, Brad King wrote: > On 06/16/2011 04:15 PM, Alexander Neundorf wrote: > > I'll push a branch to the stage once 2.8.5 is released. Or can I do that > > earlier ? > > You can push it any time but skip merging it. There should be a branch DisableSwitchForFindPackage now in stage. 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] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/16/2011 04:15 PM, Alexander Neundorf wrote: > I'll push a branch to the stage once 2.8.5 is released. Or can I do that > earlier ? You can push it any time but skip merging it. Don't expect it to be in 2.8.5 though ;) Thanks, -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Thursday 09 June 2011, Brad King wrote: > On 6/9/2011 8:50 AM, Alexander Neundorf wrote: ... > > I think this can be handled. > > find_package() should error out in this case, because Bar was required > > but it was disabled. > > Maybe this option to disable a find_package() could even be provided for > > all find_package() calls, and for each REQUIRED one it will cause an > > error. This would create a bunch of unusable options, but would be very > > consistent ;-) > > Okay. However, the option does not need to be "provided" as a > gui-settable value for packagers to be able to disable things from build > scripts. The command could honor the value if it is present but not > advertise it. I'll push a branch to the stage once 2.8.5 is released. Or can I do that earlier ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Thu, Jun 9, 2011 at 9:19 AM, Brad King wrote: > On 6/9/2011 8:50 AM, Alexander Neundorf wrote: >>> What if the FindFoo.cmake script calls find_package(Bar) and does >>> not require it but the project also does find_package(Bar) and does? I'm >>> sure there are more cases I haven't listed here. >> >> I think this can be handled. >> find_package() should error out in this case, because Bar was required but it >> was disabled. >> Maybe this option to disable a find_package() could even be provided for all >> find_package() calls, and for each REQUIRED one it will cause an error. This >> would create a bunch of unusable options, but would be very consistent ;-) > > Okay. However, the option does not need to be "provided" as a > gui-settable value for packagers to be able to disable things from build > scripts. The command could honor the value if it is present but not > advertise it. I think (as a former packager) that this would certainly go a long way to addressing the issue for packages. If the packager passes in --disable-find=Qt4 (or whatever command line syntax you use to expose) then all calls to find_package(Qt4) fail, and you just made CMake easier for packagers to use with little effort for developers in changing their scripts. Marcus ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 6/9/2011 8:50 AM, Alexander Neundorf wrote: > Calling FindQt4 in some directory with components A and B, and in some other > directory with component C, and the second one doesn't do anything because > the > first one has already set something to "I am done" already so the second one > doesn't do anything. Yeah, that's a per-package implementation detail which is not well-handled in all the scripts. > Maybe we have the same issue when somebody calls > find_package(Foo 2.0.0 NO_MODULE) > in some directory, and it finds eg. version 2.0, > and > find_package(Foo 3.0.0 NO_MODULE) > in some other directory ? > Foo_DIR (which is in the cache) has been already set then. > This could be also mixed with different components, which may are only > installed for one of the two versions... Yes, but this is not the same as finding the same version twice with different options. Mixing third-party dependency versions within the same project is hard for more reasons than this. >> What if the FindFoo.cmake script calls find_package(Bar) and does >> not require it but the project also does find_package(Bar) and does? I'm >> sure there are more cases I haven't listed here. > > I think this can be handled. > find_package() should error out in this case, because Bar was required but it > was disabled. > Maybe this option to disable a find_package() could even be provided for all > find_package() calls, and for each REQUIRED one it will cause an error. This > would create a bunch of unusable options, but would be very consistent ;-) Okay. However, the option does not need to be "provided" as a gui-settable value for packagers to be able to disable things from build scripts. The command could honor the value if it is present but not advertise it. >> That kind of auto-cache-cleanup logic will be problematic in non-toy >> cases. If two different find_package() calls lead transitively to >> the same cache values then one could erase the other's results. > > Are you sure ? I think there will be all kinds of interactions that we cannot predict leading to mysterious hard-to-find behavior. The stated goal of this feature is for packagers to disable things. That does not require any action when the state of an option changes because packagers use one-shot build scripts. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Thursday 09 June 2011, Brad King wrote:
> On 6/9/2011 2:58 AM, Alexander Neundorf wrote:
> > This wish comes mainly from packagers, not from the developers
> > themselves. I am sure packagers would be happy if they had one
> > consistent way to disable every package any cmake checks for with a
> > standardized option.
>
> This is a nice goal, but I do not think it is easy to do.
>
> What if find_package(Foo) is called in more than one location with
> different COMPONENT values? What if one is required and the other is
> not?
Now that you say it, we (cmake) have issues with this anyway.
Some people told me in Randa that they have problems with this.
Calling FindQt4 in some directory with components A and B, and in some other
directory with component C, and the second one doesn't do anything because the
first one has already set something to "I am done" already so the second one
doesn't do anything.
Maybe we have the same issue when somebody calls
find_package(Foo 2.0.0 NO_MODULE)
in some directory, and it finds eg. version 2.0,
and
find_package(Foo 3.0.0 NO_MODULE)
in some other directory ?
Foo_DIR (which is in the cache) has been already set then.
This could be also mixed with different components, which may are only
installed for one of the two versions...
Maybe, if Foo is disabled, this could be interpreted as "the user wants to be
independent from Foo, regardless of components etc.", i.e. a quite global off-
switch.
I have to think a bit more about this.
> What if the FindFoo.cmake script calls find_package(Bar) and does
> not require it but the project also does find_package(Bar) and does? I'm
> sure there are more cases I haven't listed here.
I think this can be handled.
find_package() should error out in this case, because Bar was required but it
was disabled.
Maybe this option to disable a find_package() could even be provided for all
find_package() calls, and for each REQUIRED one it will cause an error. This
would create a bunch of unusable options, but would be very consistent ;-)
> The overall problem is that not all calls of find_package() are intended
> to declare a dependency of the main project on a third party package.
> In order to make such intention explicit you need to have a different
> syntax, like a macro that handles it in a way that makes sense for a
> given project. When projects start nesting inside one another I'm not
> even sure it is a well-defined problem.
>
> > if (! mf->IsOn(DISABLE_FIND_PACKAGE_)
> > {
> >
> > cacheVars = getAllCacheVars();
> > doNormalFinding();
> > newCacheVars = getAllCacheVars();
> > addedCacheVars = newCacheVars - cacheVars;
> > FIND_PACKAGE__CACHE_VARS = addedCacheVars;
> > put FIND_PACKAGE__CACHE_VARS in the cache
> >
> > }
> > else // it is disabled
> > {
> >
> > get FIND_PACKAGE__CACHE_VARS from cache
> > remove all variables listed there from cache
> > set _FOUND to FALSE
> >
> > }
>
> That kind of auto-cache-cleanup logic will be problematic in non-toy
> cases. If two different find_package() calls lead transitively to
> the same cache values then one could erase the other's results.
Are you sure ?
Let's say I have a project which uses PNG and ZLIB.
Assume I disable PNG (which finds ZLIB).
If PNG is searched first, both PNG and ZLIB variables are remved from the
cache. After this, when ZLIB is searched, it will search again, and the
results will end up in the cache.
The other way round, if I disable ZLIB, PNG will not be found, which is ok,
since this would have dragged in ZLIB.
Or maybe not actually remove them from the cache, but set the "normal"
variables to NOTFOUND ?
Alex
___
cmake-developers mailing list
cmake-developers@cmake.org
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 6/9/2011 2:58 AM, Alexander Neundorf wrote:
> This wish comes mainly from packagers, not from the developers themselves.
> I am sure packagers would be happy if they had one consistent way to disable
> every package any cmake checks for with a standardized option.
This is a nice goal, but I do not think it is easy to do.
What if find_package(Foo) is called in more than one location with
different COMPONENT values? What if one is required and the other is
not? What if the FindFoo.cmake script calls find_package(Bar) and does
not require it but the project also does find_package(Bar) and does? I'm
sure there are more cases I haven't listed here.
The overall problem is that not all calls of find_package() are intended
to declare a dependency of the main project on a third party package.
In order to make such intention explicit you need to have a different
syntax, like a macro that handles it in a way that makes sense for a
given project. When projects start nesting inside one another I'm not
even sure it is a well-defined problem.
> if (! mf->IsOn(DISABLE_FIND_PACKAGE_)
> {
> cacheVars = getAllCacheVars();
> doNormalFinding();
> newCacheVars = getAllCacheVars();
> addedCacheVars = newCacheVars - cacheVars;
> FIND_PACKAGE__CACHE_VARS = addedCacheVars;
> put FIND_PACKAGE__CACHE_VARS in the cache
> }
> else // it is disabled
> {
> get FIND_PACKAGE__CACHE_VARS from cache
> remove all variables listed there from cache
> set _FOUND to FALSE
> }
That kind of auto-cache-cleanup logic will be problematic in non-toy
cases. If two different find_package() calls lead transitively to
the same cache values then one could erase the other's results.
-Brad
___
cmake-developers mailing list
cmake-developers@cmake.org
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Wednesday 08 June 2011, Brad King wrote:
> On 6/8/2011 2:08 PM, Alexander Neundorf wrote:
> > This would make the options available for cmake-based projects more
> > consistent.
> > This issue comes up regularly from new users.
>
> The option() command adds options. The add_subdirectory command
> adds subdirectories. The find_package command finds packages.
> These are well-defined primitives. I don't want to change this.
>
> http://en.wikipedia.org/wiki/Law_of_Demeter
>
> > So, I don't see a reason why such an option should not be the default
> > behaviour and why I should rely on an external extension.
>
> With that goal in mind I see no reason not to do this in a module
> that comes with CMake:
>
> include(OptionalFindPackage)
> optional_find_package(XYZ)
But I still don't see a reason why I should not use optional_find_package()
and use find_package(...without REQUIRED) instead. And there are good reasons
to use the optional one always if the package is not REQUIRED.
This wish comes mainly from packagers, not from the developers themselves.
I am sure packagers would be happy if they had one consistent way to disable
every package any cmake checks for with a standardized option.
Implementation could look something like
cmFindPackage::cmFindPackage()
{
if (! mf->IsOn(DISABLE_FIND_PACKAGE_)
{
cacheVars = getAllCacheVars();
doNormalFinding();
newCacheVars = getAllCacheVars();
addedCacheVars = newCacheVars - cacheVars;
FIND_PACKAGE__CACHE_VARS = addedCacheVars;
put FIND_PACKAGE__CACHE_VARS in the cache
}
else // it is disabled
{
get FIND_PACKAGE__CACHE_VARS from cache
remove all variables listed there from cache
set _FOUND to FALSE
}
}
We could put this functionality into the to-be-created extra-cmake-modules
package, or how it will be called, but after some thinking I thought it
benefits cmake much more if this option would be always available for all non-
REQUIRED find_package() calls without the developer having to care about it.
Alex
___
cmake-developers mailing list
cmake-developers@cmake.org
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Thursday 09 June 2011, Nicolas Desprès wrote: > On Wed, Jun 8, 2011 at 8:08 PM, Alexander Neundorf wrote: ... > > I can't think of any reason why somebody would want to use > > find_package(...without REQUIRED) instead of optional_find_package(). > > > > Can somebody else see a reason ? > > I have to confess that I never called find_package() without REQUIRED, > and I can't think about any use case right now. E.g. in KDE many packages are optional, if they are not found, the program is built without the feature which that package would have provided. The same for many other projects. E.g. Qt too has a lot of switches at configure-time. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
Nicolas Desprès wrote: > I have to confess that I never called find_package() without REQUIRED, > and I can't think about any use case right now. The most simple one is probably the system-or-bundled one: check if the system has a good version of some package, use the one bundled with the sources otherwise. Or just: build without some feature if a package is not availble. Eike signature.asc Description: This is a digitally signed message part. ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Wed, Jun 8, 2011 at 8:08 PM, Alexander Neundorf wrote: > On Tuesday, June 07, 2011 02:34:06 PM Eric Noulard wrote: >> 2011/6/7 Alexander Neundorf : >> > On Monday, June 06, 2011 03:26:03 PM Brad King wrote: >> >> On 06/04/2011 06:30 AM, Alexander Neundorf wrote: >> [...] >> >> >> > What do you think about adding the keyword OPTIONAL to >> >> > add_subdirectory ? >> >> > >> >> > Both have been proven useful, the one for find_package() especially >> >> > for packagers. >> >> >> >> Ditto previous response. These commands are primitives. We should not >> >> extend them with features unrelated to their basic purpose. >> > >> > While this is correct, it also keeps cmake a bit low-level. >> > For this reason, we created such macros in KDE. >> > Now our developers write stuff outside KDE, so either they can't use it, >> > or they create copies of these files. >> > This may be ok, but having 50 or 100 versions of these files and macros >> > around in the net, some probably differing slightly, is also not a nice >> > situation. >> >> Then it is possible to create a new CMake module, >> >> say >> >> UseEnhancedConfigure.cmake >> >> which could be included in CMake as a contributed module maintained by KDE >> dev. This new module would define something like: >> >> optional_find_package(). >> optional_add_subdirectory() >> >> this would make the feature available upstream, thus available outside KDE >> and does not add extra feature to builtin configure. > > I can't think of any reason why somebody would want to use > find_package(...without REQUIRED) instead of optional_find_package(). > > Can somebody else see a reason ? > I have to confess that I never called find_package() without REQUIRED, and I can't think about any use case right now. -Nico ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 6/8/2011 2:08 PM, Alexander Neundorf wrote: > This would make the options available for cmake-based projects more > consistent. > This issue comes up regularly from new users. The option() command adds options. The add_subdirectory command adds subdirectories. The find_package command finds packages. These are well-defined primitives. I don't want to change this. http://en.wikipedia.org/wiki/Law_of_Demeter > So, I don't see a reason why such an option should not be the default > behaviour and why I should rely on an external extension. With that goal in mind I see no reason not to do this in a module that comes with CMake: include(OptionalFindPackage) optional_find_package(XYZ) This will keep the additional features out of the main command and provide a separate place to document them. If it turns out that the way the feature is implemented isn't that clean we can easily create a new macro with a new name. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Wednesday, June 08, 2011 08:45:56 PM Eric Noulard wrote: > 2011/6/8 Alexander Neundorf : > > On Tuesday, June 07, 2011 02:34:06 PM Eric Noulard wrote: > >> 2011/6/7 Alexander Neundorf : > >> > On Monday, June 06, 2011 03:26:03 PM Brad King wrote: > >> >> On 06/04/2011 06:30 AM, Alexander Neundorf wrote: > >> [...] > >> > >> >> > What do you think about adding the keyword OPTIONAL to > >> >> > add_subdirectory ? > >> >> > > >> >> > Both have been proven useful, the one for find_package() especially > >> >> > for packagers. > >> >> > >> >> Ditto previous response. These commands are primitives. We should > >> >> not extend them with features unrelated to their basic purpose. > >> > > >> > While this is correct, it also keeps cmake a bit low-level. > >> > For this reason, we created such macros in KDE. > >> > Now our developers write stuff outside KDE, so either they can't use > >> > it, or they create copies of these files. > >> > This may be ok, but having 50 or 100 versions of these files and > >> > macros around in the net, some probably differing slightly, is also > >> > not a nice situation. > >> > >> Then it is possible to create a new CMake module, > >> > >> say > >> > >> UseEnhancedConfigure.cmake > >> > >> which could be included in CMake as a contributed module maintained by > >> KDE dev. This new module would define something like: > >> > >> optional_find_package(). > >> optional_add_subdirectory() > >> > >> this would make the feature available upstream, thus available outside > >> KDE and does not add extra feature to builtin configure. > > > > I can't think of any reason why somebody would want to use > > find_package(...without REQUIRED) instead of optional_find_package(). > > > > Can somebody else see a reason ? > > Our e-mails crossed each-other. > > Yes you are right default behavior of making find_package(...without > REQUIRED) This is a different story for add_subdirectory. Yes, for me adding this to find_package() is the more important and more general useful one. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
2011/6/8 Alexander Neundorf : > On Tuesday, June 07, 2011 02:34:06 PM Eric Noulard wrote: >> 2011/6/7 Alexander Neundorf : >> > On Monday, June 06, 2011 03:26:03 PM Brad King wrote: >> >> On 06/04/2011 06:30 AM, Alexander Neundorf wrote: >> [...] >> >> >> > What do you think about adding the keyword OPTIONAL to >> >> > add_subdirectory ? >> >> > >> >> > Both have been proven useful, the one for find_package() especially >> >> > for packagers. >> >> >> >> Ditto previous response. These commands are primitives. We should not >> >> extend them with features unrelated to their basic purpose. >> > >> > While this is correct, it also keeps cmake a bit low-level. >> > For this reason, we created such macros in KDE. >> > Now our developers write stuff outside KDE, so either they can't use it, >> > or they create copies of these files. >> > This may be ok, but having 50 or 100 versions of these files and macros >> > around in the net, some probably differing slightly, is also not a nice >> > situation. >> >> Then it is possible to create a new CMake module, >> >> say >> >> UseEnhancedConfigure.cmake >> >> which could be included in CMake as a contributed module maintained by KDE >> dev. This new module would define something like: >> >> optional_find_package(). >> optional_add_subdirectory() >> >> this would make the feature available upstream, thus available outside KDE >> and does not add extra feature to builtin configure. > > I can't think of any reason why somebody would want to use > find_package(...without REQUIRED) instead of optional_find_package(). > > Can somebody else see a reason ? Our e-mails crossed each-other. Yes you are right default behavior of making find_package(...without REQUIRED) This is a different story for add_subdirectory. >> > These two OPTION features in this email are IMO features which are useful >> > in many projects and which would make using cmake-based projects easier >> > for users (people compiling the software), since they could expect that >> > if packages can be disabled, that this will be done via an option with a >> > name which always follows the same scheme. >> >> I totally agree with that but I think it does not implies this has to be >> done by builtin find_package/add_subdirectory ? > > Well, it doesn't have to. > But if packagers requested this for something in KDE, they will also want this > for non-KDE projects. > And I see no reason why this shouldn't be the default for _every_ non-REQUIRED > package. > This would make the options available for cmake-based projects more > consistent. > This issue comes up regularly from new users. > > So, I don't see a reason why such an option should not be the default > behaviour and why I should rely on an external extension. Again I agree for find_package since the extra behavior seems backward compatible, (I need more time to think about it) you'll have to take care of name collision with existing user defined OPTION though. -- 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] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Jun 8, 2011, at 12:08 PM, Alexander Neundorf wrote:
> On Tuesday, June 07, 2011 02:34:06 PM Eric Noulard wrote:
>> 2011/6/7 Alexander Neundorf :
>>> On Monday, June 06, 2011 03:26:03 PM Brad King wrote:
On 06/04/2011 06:30 AM, Alexander Neundorf wrote:
>> [...]
>>
> What do you think about adding the keyword OPTIONAL to
> add_subdirectory ?
>
> Both have been proven useful, the one for find_package() especially
> for packagers.
Ditto previous response. These commands are primitives. We should not
extend them with features unrelated to their basic purpose.
>>>
>>> While this is correct, it also keeps cmake a bit low-level.
>>> For this reason, we created such macros in KDE.
>>> Now our developers write stuff outside KDE, so either they can't use it,
>>> or they create copies of these files.
>>> This may be ok, but having 50 or 100 versions of these files and macros
>>> around in the net, some probably differing slightly, is also not a nice
>>> situation.
>>
>> Then it is possible to create a new CMake module,
>>
>> say
>>
>> UseEnhancedConfigure.cmake
>>
>> which could be included in CMake as a contributed module maintained by KDE
>> dev. This new module would define something like:
>>
>> optional_find_package().
>> optional_add_subdirectory()
>>
>> this would make the feature available upstream, thus available outside KDE
>> and does not add extra feature to builtin configure.
>
> I can't think of any reason why somebody would want to use
> find_package(...without REQUIRED) instead of optional_find_package().
>
> Can somebody else see a reason ?
>
>
>>> These two OPTION features in this email are IMO features which are useful
>>> in many projects and which would make using cmake-based projects easier
>>> for users (people compiling the software), since they could expect that
>>> if packages can be disabled, that this will be done via an option with a
>>> name which always follows the same scheme.
>>
>> I totally agree with that but I think it does not implies this has to be
>> done by builtin find_package/add_subdirectory ?
>
> Well, it doesn't have to.
> But if packagers requested this for something in KDE, they will also want
> this
> for non-KDE projects.
> And I see no reason why this shouldn't be the default for _every_
> non-REQUIRED
> package.
> This would make the options available for cmake-based projects more
> consistent.
> This issue comes up regularly from new users.
>
> So, I don't see a reason why such an option should not be the default
> behaviour and why I should rely on an external extension.
Just something that came to mind... what if one wanted one cmake option that
corresponds with multiple find_package()'s.
But then wouldn't the following work for that case?
option(MYOPTION... )
set(FOO_OPTION ${MYOPTION} CACHE INTERNAL "")
set(BAR_OPTION ${MYOPTION} CACHE INTERNAL "")
find_package(FOO)
find_package(BAR)
Clint
___
cmake-developers mailing list
cmake-developers@cmake.org
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Tuesday, June 07, 2011 02:34:06 PM Eric Noulard wrote: > 2011/6/7 Alexander Neundorf : > > On Monday, June 06, 2011 03:26:03 PM Brad King wrote: > >> On 06/04/2011 06:30 AM, Alexander Neundorf wrote: > [...] > > >> > What do you think about adding the keyword OPTIONAL to > >> > add_subdirectory ? > >> > > >> > Both have been proven useful, the one for find_package() especially > >> > for packagers. > >> > >> Ditto previous response. These commands are primitives. We should not > >> extend them with features unrelated to their basic purpose. > > > > While this is correct, it also keeps cmake a bit low-level. > > For this reason, we created such macros in KDE. > > Now our developers write stuff outside KDE, so either they can't use it, > > or they create copies of these files. > > This may be ok, but having 50 or 100 versions of these files and macros > > around in the net, some probably differing slightly, is also not a nice > > situation. > > Then it is possible to create a new CMake module, > > say > > UseEnhancedConfigure.cmake > > which could be included in CMake as a contributed module maintained by KDE > dev. This new module would define something like: > > optional_find_package(). > optional_add_subdirectory() > > this would make the feature available upstream, thus available outside KDE > and does not add extra feature to builtin configure. I can't think of any reason why somebody would want to use find_package(...without REQUIRED) instead of optional_find_package(). Can somebody else see a reason ? > > These two OPTION features in this email are IMO features which are useful > > in many projects and which would make using cmake-based projects easier > > for users (people compiling the software), since they could expect that > > if packages can be disabled, that this will be done via an option with a > > name which always follows the same scheme. > > I totally agree with that but I think it does not implies this has to be > done by builtin find_package/add_subdirectory ? Well, it doesn't have to. But if packagers requested this for something in KDE, they will also want this for non-KDE projects. And I see no reason why this shouldn't be the default for _every_ non-REQUIRED package. This would make the options available for cmake-based projects more consistent. This issue comes up regularly from new users. So, I don't see a reason why such an option should not be the default behaviour and why I should rely on an external extension. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
2011/6/7 Eric Noulard : > 2011/6/7 Alexander Neundorf : >> On Monday, June 06, 2011 03:26:03 PM Brad King wrote: >>> On 06/04/2011 06:30 AM, Alexander Neundorf wrote: > [...] > >>> > >>> > What do you think about adding the keyword OPTIONAL to add_subdirectory ? >>> > >>> > Both have been proven useful, the one for find_package() especially for >>> > packagers. >>> >>> Ditto previous response. These commands are primitives. We should not >>> extend them with features unrelated to their basic purpose. >> >> While this is correct, it also keeps cmake a bit low-level. >> For this reason, we created such macros in KDE. >> Now our developers write stuff outside KDE, so either they can't use it, or >> they create copies of these files. >> This may be ok, but having 50 or 100 versions of these files and macros >> around >> in the net, some probably differing slightly, is also not a nice situation. > > Then it is possible to create a new CMake module, > > say > > UseEnhancedConfigure.cmake I meant UseEnhancedFindPackage but I think you get the idea. > > which could be included in CMake as a contributed module maintained by KDE > dev. > This new module would define something like: > > optional_find_package(). > optional_add_subdirectory() > > this would make the feature available upstream, thus available outside KDE > and does not add extra feature to builtin configure. > >> >> These two OPTION features in this email are IMO features which are useful in >> many projects and which would make using cmake-based projects easier for >> users >> (people compiling the software), since they could expect that if packages can >> be disabled, that this will be done via an option with a name which always >> follows the same scheme. > > I totally agree with that but I think it does not implies this has to be > done by builtin find_package/add_subdirectory ? > > -- > Erk > Membre de l'April - « promouvoir et défendre le logiciel libre » - > http://www.april.org > -- 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] Adding argument "OPTIONAL" to find_package() and add_subdirectory
2011/6/7 Alexander Neundorf : > On Monday, June 06, 2011 03:26:03 PM Brad King wrote: >> On 06/04/2011 06:30 AM, Alexander Neundorf wrote: [...] >> > >> > What do you think about adding the keyword OPTIONAL to add_subdirectory ? >> > >> > Both have been proven useful, the one for find_package() especially for >> > packagers. >> >> Ditto previous response. These commands are primitives. We should not >> extend them with features unrelated to their basic purpose. > > While this is correct, it also keeps cmake a bit low-level. > For this reason, we created such macros in KDE. > Now our developers write stuff outside KDE, so either they can't use it, or > they create copies of these files. > This may be ok, but having 50 or 100 versions of these files and macros around > in the net, some probably differing slightly, is also not a nice situation. Then it is possible to create a new CMake module, say UseEnhancedConfigure.cmake which could be included in CMake as a contributed module maintained by KDE dev. This new module would define something like: optional_find_package(). optional_add_subdirectory() this would make the feature available upstream, thus available outside KDE and does not add extra feature to builtin configure. > > These two OPTION features in this email are IMO features which are useful in > many projects and which would make using cmake-based projects easier for users > (people compiling the software), since they could expect that if packages can > be disabled, that this will be done via an option with a name which always > follows the same scheme. I totally agree with that but I think it does not implies this has to be done by builtin find_package/add_subdirectory ? -- 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] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Monday, June 06, 2011 03:26:03 PM Brad King wrote:
> On 06/04/2011 06:30 AM, Alexander Neundorf wrote:
> > What do you think about adding this as a built-in feature to
> > find_package(), i.e. add a argument OPTIONAL to find_package(), then
> > probably also a "COMMENT".
>
> The interface to find_package is already extensive. This feature would
> have to come with a whole bunch of other options to control the name of
> the variable to match project conventions, etc...and none of these has
> anything to do with actually finding the package. These features belong
> in project-specific macros, or simply
>
> option(USE_FOO "Enable features that require FOO")
> if(USE_FOO)
> find_package(FOO)
> endif()
>
> which IMO is more readable anyway.
It is 4 lines of cmake code to write instead of 1.
If this is done for several find_package() calls in a row this adds up.
Having such an option built-in would also hint the developer what the
recommended way to disable packages is.
E.g. with configure-scripts there is usually a --disable-foo switch.
With cmake this is completely up to the developers how to do it.
Having such an option for find_package() could establish such a standard way
how to disable a library/package in cmake-projects.
Additionally, the four lines are not enough.
Later on in the CMakeLists.txt there are usually checks whether FOO has been
found.
So at least FOO_FOUND has to be reset to FALSE, so a minimal version would be:
option(USE_FOO "Enable features that require FOO")
if(USE_FOO)
find_package(FOO)
else()
set(FOO_FOUND FALSE)
endif()
It also happens that developers test e.g. for FOO_LIBRARY etc.
Which makes the macro currently look like the following:
macro(_MOFP_SET_EMPTY_IF_DEFINED _name _var)
if(DEFINED ${_name}_${_var})
set(${_name}_${_var} "")
endif(DEFINED ${_name}_${_var})
string(TOUPPER ${_name} _nameUpper)
if(DEFINED ${_nameUpper}_${_var})
set(${_nameUpper}_${_var} "")
endif(DEFINED ${_nameUpper}_${_var})
endmacro(_MOFP_SET_EMPTY_IF_DEFINED _package _var)
macro (MACRO_OPTIONAL_FIND_PACKAGE _name )
option(WITH_${_name} "Search for ${_name} package" ON)
if (WITH_${_name})
find_package(${_name} ${ARGN})
else (WITH_${_name})
string(TOUPPER ${_name} _nameUpper)
set(${_name}_FOUND FALSE)
set(${_nameUpper}_FOUND FALSE)
_mofp_set_empty_if_defined(${_name} INCLUDE_DIRS)
_mofp_set_empty_if_defined(${_name} INCLUDE_DIR)
_mofp_set_empty_if_defined(${_name} INCLUDES)
_mofp_set_empty_if_defined(${_name} LIBRARY)
_mofp_set_empty_if_defined(${_name} LIBRARIES)
_mofp_set_empty_if_defined(${_name} LIBS)
_mofp_set_empty_if_defined(${_name} FLAGS)
_mofp_set_empty_if_defined(${_name} DEFINITIONS)
endif (WITH_${_name})
endmacro (MACRO_OPTIONAL_FIND_PACKAGE)
> > 2) we have something similar for add_subdirectory(),
> > macro_optional_add_subdirectory().
> > The purpose is to disable parts of a big project by skipping the
> > add_subdirectory.
> >
> > This is a wrapper around add_subdirectory(), but adds an option
> > BUILD_FOO, which is enabled by default.
> > If disabled, the actual add_subdirectory() is not executed.
> > Additionally there is an additional global option
> > DISABLE_ALL_OPTIONAL_SUBDIRECTORIES, which is disabled by default.
> > If enabled, all these optional subdirectories are disabled (but can be
> > enabled again one by one).
> >
> > What do you think about adding the keyword OPTIONAL to add_subdirectory ?
> >
> > Both have been proven useful, the one for find_package() especially for
> > packagers.
>
> Ditto previous response. These commands are primitives. We should not
> extend them with features unrelated to their basic purpose.
While this is correct, it also keeps cmake a bit low-level.
For this reason, we created such macros in KDE.
Now our developers write stuff outside KDE, so either they can't use it, or
they create copies of these files.
This may be ok, but having 50 or 100 versions of these files and macros around
in the net, some probably differing slightly, is also not a nice situation.
These two OPTION features in this email are IMO features which are useful in
many projects and which would make using cmake-based projects easier for users
(people compiling the software), since they could expect that if packages can
be disabled, that this will be done via an option with a name which always
follows the same scheme.
Alex
___
cmake-developers mailing list
cmake-developers@cmake.org
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On 06/04/2011 06:30 AM, Alexander Neundorf wrote: > What do you think about adding this as a built-in feature to find_package(), > i.e. add a argument OPTIONAL to find_package(), then probably also a > "COMMENT". The interface to find_package is already extensive. This feature would have to come with a whole bunch of other options to control the name of the variable to match project conventions, etc...and none of these has anything to do with actually finding the package. These features belong in project-specific macros, or simply option(USE_FOO "Enable features that require FOO") if(USE_FOO) find_package(FOO) endif() which IMO is more readable anyway. > 2) we have something similar for add_subdirectory(), > macro_optional_add_subdirectory(). > The purpose is to disable parts of a big project by skipping the > add_subdirectory. > > This is a wrapper around add_subdirectory(), but adds an option BUILD_FOO, > which is enabled by default. > If disabled, the actual add_subdirectory() is not executed. > Additionally there is an additional global option > DISABLE_ALL_OPTIONAL_SUBDIRECTORIES, which is disabled by default. > If enabled, all these optional subdirectories are disabled (but can be > enabled > again one by one). > > What do you think about adding the keyword OPTIONAL to add_subdirectory ? > > Both have been proven useful, the one for find_package() especially for > packagers. Ditto previous response. These commands are primitives. We should not extend them with features unrelated to their basic purpose. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Sun, Jun 5, 2011 at 4:44 PM, Alexander Neundorf wrote: > On Sunday, June 05, 2011 11:50:50 PM Eric Noulard wrote: > > 2011/6/4 Alexander Neundorf : > > > Hi, > > > > > > again from the KDE sprint... > > > > > > 1) We have a macro > > > macro_optional_find_package(). > > > The purpose is to be able to build without a specific package even if > > > that package is installed and would actually be found by the > > > find_package() call. > > > > > > Basically this is a wrapper around find_package(), but additionally it > > > adds an option WITH_Foo, which is enabled by default. > > > If disabled, the actual find_package() is not executed, and the > variables > > > FOO_FOUND/INCLUDES/LIBRARIES are reset to empty, so that even if the > > > package hasn't been searched this time, results from previous runs are > > > discarded. > > > > > > What do you think about adding this as a built-in feature to > > > find_package(), i.e. add a argument OPTIONAL to find_package(), then > > > probably also a "COMMENT". > > > > I'm not sure about this feature and since we already have REQUIRED why > > would be the meaning of OPTIONAL? > > Using OPTIONAL and REQUIRED together should be considered an error. > OPTIONAL means you can disable the searching so that it will also not be > "found" even if it is actually installed. > > > May be some syntax like > > > > find_package(Foo IF ) would be clearer. > > > > find_package(Foo IF BUILD_WITH_Foo) > > > > would create option BUILD_WITH_Foo > > (default value would the value of BUILD_WITH_Foo_Default var or ON if > > it is not defined) > > and the search of the package will be done iff the option is ON. > > Maybe. > I thought OPTIONAL would be good since it hints at "OPTION", and it is an > option which is created for disabling it. > Could also be "OPTION" instead of "OPTIONAL". > > ... > > > Both have been proven useful, the one for find_package() especially for > > > packagers. > > > > Agreed, the feature is nice I use similar thing with hand-crafted > > option in some project. > > Alex > ___ > cmake-developers mailing list > cmake-developers@cmake.org > http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers > I think the use of OPTIONAL to specify "Add CMake option to not look for this package" kind of confusing. I would prefer a more verbose, but clear flag. Something like: ADD_CMAKE_OPTION_TO_DISABLE James ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
2011/6/6 Alexander Neundorf : > On Sunday, June 05, 2011 11:50:50 PM Eric Noulard wrote: >> 2011/6/4 Alexander Neundorf : >> > Hi, >> > >> > again from the KDE sprint... >> > >> > 1) We have a macro >> > macro_optional_find_package(). >> > The purpose is to be able to build without a specific package even if >> > that package is installed and would actually be found by the >> > find_package() call. >> > >> > Basically this is a wrapper around find_package(), but additionally it >> > adds an option WITH_Foo, which is enabled by default. >> > If disabled, the actual find_package() is not executed, and the variables >> > FOO_FOUND/INCLUDES/LIBRARIES are reset to empty, so that even if the >> > package hasn't been searched this time, results from previous runs are >> > discarded. >> > >> > What do you think about adding this as a built-in feature to >> > find_package(), i.e. add a argument OPTIONAL to find_package(), then >> > probably also a "COMMENT". >> >> I'm not sure about this feature and since we already have REQUIRED why >> would be the meaning of OPTIONAL? > > Using OPTIONAL and REQUIRED together should be considered an error. > OPTIONAL means you can disable the searching so that it will also not be > "found" even if it is actually installed. Yes I think get your point. I was just thinking that not specifying REQUIRED already means OPTIONAL whereas specifying OPTIONAL means "could be disabled". >> May be some syntax like >> >> find_package(Foo IF ) would be clearer. >> >> find_package(Foo IF BUILD_WITH_Foo) >> >> would create option BUILD_WITH_Foo >> (default value would the value of BUILD_WITH_Foo_Default var or ON if >> it is not defined) >> and the search of the package will be done iff the option is ON. > > Maybe. > I thought OPTIONAL would be good since it hints at "OPTION", and it is an > option which is created for disabling it. > Could also be "OPTION" instead of "OPTIONAL". I'd prefer OPTION over OPTIONAL I think it's less confusing with REQUIRED. My idea with IF was a little broader than OPTION you could feed IF argument with an existing var (not necessarily an option) and the package search would be done if the var evaluate to true/on/yes That way one can encode several ways of enable/disabling a find_package subset. If no var is defined then find_package may create an option using this name with a default value set to ON. However, may be the IF idea is simply overkill. -- 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] Adding argument "OPTIONAL" to find_package() and add_subdirectory
On Sunday, June 05, 2011 11:50:50 PM Eric Noulard wrote: > 2011/6/4 Alexander Neundorf : > > Hi, > > > > again from the KDE sprint... > > > > 1) We have a macro > > macro_optional_find_package(). > > The purpose is to be able to build without a specific package even if > > that package is installed and would actually be found by the > > find_package() call. > > > > Basically this is a wrapper around find_package(), but additionally it > > adds an option WITH_Foo, which is enabled by default. > > If disabled, the actual find_package() is not executed, and the variables > > FOO_FOUND/INCLUDES/LIBRARIES are reset to empty, so that even if the > > package hasn't been searched this time, results from previous runs are > > discarded. > > > > What do you think about adding this as a built-in feature to > > find_package(), i.e. add a argument OPTIONAL to find_package(), then > > probably also a "COMMENT". > > I'm not sure about this feature and since we already have REQUIRED why > would be the meaning of OPTIONAL? Using OPTIONAL and REQUIRED together should be considered an error. OPTIONAL means you can disable the searching so that it will also not be "found" even if it is actually installed. > May be some syntax like > > find_package(Foo IF ) would be clearer. > > find_package(Foo IF BUILD_WITH_Foo) > > would create option BUILD_WITH_Foo > (default value would the value of BUILD_WITH_Foo_Default var or ON if > it is not defined) > and the search of the package will be done iff the option is ON. Maybe. I thought OPTIONAL would be good since it hints at "OPTION", and it is an option which is created for disabling it. Could also be "OPTION" instead of "OPTIONAL". ... > > Both have been proven useful, the one for find_package() especially for > > packagers. > > Agreed, the feature is nice I use similar thing with hand-crafted > option in some project. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding argument "OPTIONAL" to find_package() and add_subdirectory
2011/6/4 Alexander Neundorf : > Hi, > > again from the KDE sprint... > > 1) We have a macro > macro_optional_find_package(). > The purpose is to be able to build without a specific package even if that > package is installed and would actually be found by the find_package() call. > > Basically this is a wrapper around find_package(), but additionally it adds an > option WITH_Foo, which is enabled by default. > If disabled, the actual find_package() is not executed, and the variables > FOO_FOUND/INCLUDES/LIBRARIES are reset to empty, so that even if the package > hasn't been searched this time, results from previous runs are discarded. > > What do you think about adding this as a built-in feature to find_package(), > i.e. add a argument OPTIONAL to find_package(), then probably also a > "COMMENT". I'm not sure about this feature and since we already have REQUIRED why would be the meaning of OPTIONAL? May be some syntax like find_package(Foo IF ) would be clearer. find_package(Foo IF BUILD_WITH_Foo) would create option BUILD_WITH_Foo (default value would the value of BUILD_WITH_Foo_Default var or ON if it is not defined) and the search of the package will be done iff the option is ON. > 2) we have something similar for add_subdirectory(), > macro_optional_add_subdirectory(). > The purpose is to disable parts of a big project by skipping the > add_subdirectory. > > This is a wrapper around add_subdirectory(), but adds an option BUILD_FOO, > which is enabled by default. > If disabled, the actual add_subdirectory() is not executed. > Additionally there is an additional global option > DISABLE_ALL_OPTIONAL_SUBDIRECTORIES, which is disabled by default. > If enabled, all these optional subdirectories are disabled (but can be enabled > again one by one). > > What do you think about adding the keyword OPTIONAL to add_subdirectory ? Same remark may IF keyword with the option name as argument would be better. > Both have been proven useful, the one for find_package() especially for > packagers. Agreed, the feature is nice I use similar thing with hand-crafted option in some project. -- 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
