Re: [CMake] New version of FindGit.cmake compatible with FindSubversion.cmake

2012-07-12 Thread Rolf Eike Beer
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

2012-07-12 Thread Mateusz Loskot
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

2012-07-12 Thread Jean-Christophe Fillion-Robin
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

2012-07-12 Thread Mateusz Loskot
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

2012-07-11 Thread Aashish Chaudhary
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

2012-07-11 Thread Mateusz Loskot
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