Feature - Add version method for git scm per #9396

[abrousseau001]

Changelog: Feature: Add Git.version property to check the current git version, aligned with SVN.version.
Docs: conan-io/docs#2338

Implements feature request #9396

This was left as just the entire return string instead of the major.minor.patch as was suggested by feature request to conform with what was already being done with the SVN side of things.

A simple test to check if the return of the version is not the ConanException error indicating the executable was missing from the path

• Refer to the issue that supports this Pull Request.
• If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
• I've read the Contributing guide.
• I've followed the PEP8 style guides for Python code.

^{Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.}

[abrousseau001]

I did not open another PR in the documentation as the version string is not documented for SVN. It seems only very high level information is given to the using of source control.

[memsharded]

I am a bit confused by this PR (more exactly, about the request). Apparently, Git already contains a get_version() method, so the main issue was that it was not public nor documented?

In that case, isn't it enough to document the method? Caching the result is not bad, but as this caching is per-instance, it wouldn't even benefit version checking from multiple recipes.

Am I missing something?
Thanks for contributing this!

[abrousseau001]

@memsharded Git itself did not have a get_version() method. That is being called from the parent class SCMBase. I modelled the way we were doing SVN to match for Git. Neither this pull request nor SVN's version property are documented. If we want to change the way this is working, I would suggest thinking a bit more on how we should implement the concrete implementations of Git and SVN as singleton classes.

[memsharded]

Git itself did not have a get_version() method.

But Git extends from SCMBase: class Git(SCMBase):, yo, yes, it has a get_version() method, inherited from SCMBase. There is even a test that validates it works:

    @patch('subprocess.Popen')
    def test_version(self, mocked_open):
        mocked_open.return_value.communicate.return_value = ('git version 2.21.0'.encode(), None)
        version = Git.get_version()
        self.assertEqual(version, "2.21.0")

[abrousseau001]

@memsharded I understand that, but why does SVN implement it the same way as a property? I think that we should either remove the SVN property and allow the SCMBase handle the version or implement this pull request in the same way. Either way is good with me, but consistency is best.

[memsharded]

@memsharded I understand that, but why does SVN implement it the same way as a property? I think that we should either remove the SVN property and allow the SCMBase handle the version or implement this pull request in the same way. Either way is good with me, but consistency is best.

Oh! Now I see what you mean, sorry I didn't understand that. Yes, this is inconsistent, and I agree that keeping consistency is best. I see SVN is actually documenting version() as a method 🤦 not a property, so we could declare a bug in the docs, and change the docs to "property" instead (while documenting the new property in Git as property too).

[memsharded]

@abrousseau001 Added conan-io/docs#2338 PR to docu for you