Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake
Mateusz Loskot wrote: On 12 July 2012 00:24, Aashish Chaudhary aashish.chaudh...@kitware.com wrote: Hi Mateusz, I don't have both files in front of me. Whats the difference between the one in the slicer and once in the CMake? CMake's current version [1] defines: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # GIT_VERSION_STRING - the version of git found (since CMake 2.8.8) Slicer's version [2] defines: # The module defines the following variables: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # # If the command line client executable is found the macro # GIT_WC_INFO(dir var-prefix) # is defined to extract information of a git working copy at # a given location. # # The macro defines the following variables: # var-prefix_WC_REVISION_HASH - Current SHA1 hash # var-prefix_WC_REVISION - Current SHA1 hash # var-prefix_WC_REVISION_NAME - Name associated with var-prefix_WC_REVISION_HASH # var-prefix_WC_URL - output of command `git config --get remote.origin.url' # var-prefix_WC_ROOT - Same value as working copy URL # var-prefix_WC_GITSVN - Set to false # # ... and also the following ones if it's a git-svn repository: # var-prefix_WC_GITSVN - Set to True if it is a # var-prefix_WC_INFO - output of command `git svn info' # var-prefix_WC_URL - url of the associated SVN repository # var-prefix_WC_ROOT - root url of the associated SVN repository # var-prefix_WC_REVISION - current SVN revision number # var-prefix_WC_LAST_CHANGED_AUTHOR - author of last commit # var-prefix_WC_LAST_CHANGED_DATE - date of last commit # var-prefix_WC_LAST_CHANGED_REV - revision of last commit # var-prefix_WC_LAST_CHANGED_LOG - last log of base revision I'm not against that change, but I find the documentation a bit misleading. When reading it my first thought was that _WC_REVISION and _WC_GITSVN are junk, just to learn later that they have different meaning when it's a git-svn repo. Looking at the code I think that line 115 (which is: if(NOT ${git_config_output} STREQUAL )) could be simplified to if(git_config_output). The warning in line 144 looks suspicious to me, that would e.g. trigger on a local only git repo, no? Maybe make this an AUTHOR_WARNING? And obviously the version extraction code is missing, but this should be trivially fixable. Greetings, Eike signature.asc Description: This is a digitally signed message part. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake
Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake
On 12 July 2012 07:52, Rolf Eike Beer e...@sf-mail.de wrote: Mateusz Loskot wrote: On 12 July 2012 00:24, Aashish Chaudhary aashish.chaudh...@kitware.com wrote: I don't have both files in front of me. Whats the difference between the one in the slicer and once in the CMake? CMake's current version [1] defines: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # GIT_VERSION_STRING - the version of git found (since CMake 2.8.8) Slicer's version [2] defines: # The module defines the following variables: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # # If the command line client executable is found the macro # GIT_WC_INFO(dir var-prefix) # is defined to extract information of a git working copy at # a given location. # # The macro defines the following variables: # var-prefix_WC_REVISION_HASH - Current SHA1 hash # var-prefix_WC_REVISION - Current SHA1 hash # var-prefix_WC_REVISION_NAME - Name associated with var-prefix_WC_REVISION_HASH # var-prefix_WC_URL - output of command `git config --get remote.origin.url' # var-prefix_WC_ROOT - Same value as working copy URL # var-prefix_WC_GITSVN - Set to false # # ... and also the following ones if it's a git-svn repository: # var-prefix_WC_GITSVN - Set to True if it is a # var-prefix_WC_INFO - output of command `git svn info' # var-prefix_WC_URL - url of the associated SVN repository # var-prefix_WC_ROOT - root url of the associated SVN repository # var-prefix_WC_REVISION - current SVN revision number # var-prefix_WC_LAST_CHANGED_AUTHOR - author of last commit # var-prefix_WC_LAST_CHANGED_DATE - date of last commit # var-prefix_WC_LAST_CHANGED_REV - revision of last commit # var-prefix_WC_LAST_CHANGED_LOG - last log of base revision I'm not against that change, but I find the documentation a bit misleading. When reading it my first thought was that _WC_REVISION and _WC_GITSVN are junk, just to learn later that they have different meaning when it's a git-svn repo. That's what documentation is for, to clarify unobvious. Looking at the code I think that line 115 (which is: if(NOT ${git_config_output} STREQUAL )) could be simplified to if(git_config_output). The warning in line 144 looks suspicious to me, that would e.g. trigger on a local only git repo, no? Maybe make this an AUTHOR_WARNING? Let's assume it is a requirement specific to the Slicer project. I think, the warning could be simply removed. And obviously the version extraction code is missing, but this should be trivially fixable. What version you mean? Git program version? Or, the TODO comment in line 71 of the Slicer's FindGit.cmake? My main point is to get someone from CMake team to simply grab the Slicer's version and commit to CMake upstream. Can't see reasons why it couldn't be submitted really. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake
Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake
Hi Folks, I also think contributing this module back to CMake would be great. This has been discuss on the list [1], few things need to be fixed and as discussed documentation updated. The module could also probably be split into: FindGit.cmake and GitInfo.cmake (or similar name) First, we could improve the module within Slicer :) [2], then, when we are all happy with it, I believe the module could probably be contributed back to CMake [3]. Hth Jc [1] http://cmake.3232098.n2.nabble.com/Addition-to-FindGit-tp6441747p6446399.html [2] http://www.slicer.org/slicerWiki/index.php/Documentation/4.1/Developers/Tutorials/ContributePatch [3] http://www.cmake.org/Wiki/CMake:Module_Maintainers On Thu, Jul 12, 2012 at 6:52 AM, Mateusz Loskot mate...@loskot.net wrote: On 12 July 2012 07:52, Rolf Eike Beer e...@sf-mail.de wrote: Mateusz Loskot wrote: On 12 July 2012 00:24, Aashish Chaudhary aashish.chaudh...@kitware.com wrote: I don't have both files in front of me. Whats the difference between the one in the slicer and once in the CMake? CMake's current version [1] defines: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # GIT_VERSION_STRING - the version of git found (since CMake 2.8.8) Slicer's version [2] defines: # The module defines the following variables: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # # If the command line client executable is found the macro # GIT_WC_INFO(dir var-prefix) # is defined to extract information of a git working copy at # a given location. # # The macro defines the following variables: # var-prefix_WC_REVISION_HASH - Current SHA1 hash # var-prefix_WC_REVISION - Current SHA1 hash # var-prefix_WC_REVISION_NAME - Name associated with var-prefix_WC_REVISION_HASH # var-prefix_WC_URL - output of command `git config --get remote.origin.url' # var-prefix_WC_ROOT - Same value as working copy URL # var-prefix_WC_GITSVN - Set to false # # ... and also the following ones if it's a git-svn repository: # var-prefix_WC_GITSVN - Set to True if it is a # var-prefix_WC_INFO - output of command `git svn info' # var-prefix_WC_URL - url of the associated SVN repository # var-prefix_WC_ROOT - root url of the associated SVN repository # var-prefix_WC_REVISION - current SVN revision number # var-prefix_WC_LAST_CHANGED_AUTHOR - author of last commit # var-prefix_WC_LAST_CHANGED_DATE - date of last commit # var-prefix_WC_LAST_CHANGED_REV - revision of last commit # var-prefix_WC_LAST_CHANGED_LOG - last log of base revision I'm not against that change, but I find the documentation a bit misleading. When reading it my first thought was that _WC_REVISION and _WC_GITSVN are junk, just to learn later that they have different meaning when it's a git-svn repo. That's what documentation is for, to clarify unobvious. Looking at the code I think that line 115 (which is: if(NOT ${git_config_output} STREQUAL )) could be simplified to if(git_config_output). The warning in line 144 looks suspicious to me, that would e.g. trigger on a local only git repo, no? Maybe make this an AUTHOR_WARNING? Let's assume it is a requirement specific to the Slicer project. I think, the warning could be simply removed. And obviously the version extraction code is missing, but this should be trivially fixable. What version you mean? Git program version? Or, the TODO comment in line 71 of the Slicer's FindGit.cmake? My main point is to get someone from CMake team to simply grab the Slicer's version and commit to CMake upstream. Can't see reasons why it couldn't be submitted really. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake -- +1 919 869 8849 -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake
Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake
On 12 July 2012 14:24, Jean-Christophe Fillion-Robin jchris.filli...@kitware.com wrote: I also think contributing this module back to CMake would be great. +1 This has been discuss on the list [1], few things need to be fixed and as discussed documentation updated. Thanks for pointing that. The module could also probably be split into: FindGit.cmake and GitInfo.cmake (or similar name) I agree, I just didn't want to make my request too bold. The FindSubversion.cmake has also contributed to the general mess and lack of unification across modules, so I assumed FindGit.cmake in current shape would be accepted. First, we could improve the module within Slicer :) [2], then, when we are all happy with it, I believe the module could probably be contributed back to CMake [3]. Or, just improve it, submit to CMake and tell the Slicer folks here is new version, if you like, use it. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake
Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake
Hi Mateusz, I don't have both files in front of me. Whats the difference between the one in the slicer and once in the CMake? Thanks On Wed, Jul 11, 2012 at 6:55 PM, Mateusz Loskot mate...@loskot.net wrote: Folks, Can we simply replace the basic version of FindGit.cmake module available in the current release and Git repo with this version from Slicer? https://github.com/Slicer/Slicer/blob/master/CMake/FindGit.cmake Git is a tool of enormous importance nowadays, as Subversion is. It's pity functionality of those two modules is kept in sync. The Slicer's version has been referred several times here anyway. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake -- | Aashish Chaudhary | RD Engineer | Kitware Inc. | www.kitware.com -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake
Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake
On 12 July 2012 00:24, Aashish Chaudhary aashish.chaudh...@kitware.com wrote: Hi Mateusz, I don't have both files in front of me. Whats the difference between the one in the slicer and once in the CMake? CMake's current version [1] defines: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # GIT_VERSION_STRING - the version of git found (since CMake 2.8.8) Slicer's version [2] defines: # The module defines the following variables: # GIT_EXECUTABLE - path to git command line client # GIT_FOUND - true if the command line client was found # # If the command line client executable is found the macro # GIT_WC_INFO(dir var-prefix) # is defined to extract information of a git working copy at # a given location. # # The macro defines the following variables: # var-prefix_WC_REVISION_HASH - Current SHA1 hash # var-prefix_WC_REVISION - Current SHA1 hash # var-prefix_WC_REVISION_NAME - Name associated with var-prefix_WC_REVISION_HASH # var-prefix_WC_URL - output of command `git config --get remote.origin.url' # var-prefix_WC_ROOT - Same value as working copy URL # var-prefix_WC_GITSVN - Set to false # # ... and also the following ones if it's a git-svn repository: # var-prefix_WC_GITSVN - Set to True if it is a # var-prefix_WC_INFO - output of command `git svn info' # var-prefix_WC_URL - url of the associated SVN repository # var-prefix_WC_ROOT - root url of the associated SVN repository # var-prefix_WC_REVISION - current SVN revision number # var-prefix_WC_LAST_CHANGED_AUTHOR - author of last commit # var-prefix_WC_LAST_CHANGED_DATE - date of last commit # var-prefix_WC_LAST_CHANGED_REV - revision of last commit # var-prefix_WC_LAST_CHANGED_LOG - last log of base revision [1] http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/FindGit.cmake;h=f89d1afa3713629b240803f9cb868dfe070771d5;hb=HEAD [2] https://github.com/Slicer/Slicer/blob/master/CMake/FindGit.cmake Best regards, -- Mateusz Loskot, http://mateusz.loskot.net -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake