version: refactor the code

- make it to use standard ISO 8601 dates
- use namedtuple for easier usage of this type
- use datetime objects for better handling of dates

Signed-off-by: Arthur Zamarin <arthurzam@gentoo.org>

3 files changed, 96 insertions, 81 deletions

diff --git a/src/snakeoil/dist/distutils_extensions.py b/src/snakeoil/dist/distutils_extensions.py
index f919826c..a51ec284 100644
--- a/src/snakeoil/dist/distutils_extensions.py
+++ b/src/snakeoil/dist/distutils_extensions.py
@@ -18,7 +18,6 @@ import subprocess
 import sys
 import textwrap
 from contextlib import ExitStack, contextmanager, redirect_stderr, redirect_stdout
-from datetime import datetime
 from multiprocessing import cpu_count
 
 from setuptools import find_packages
@@ -126,8 +125,8 @@ def module_version(moduledir=MODULEDIR):
             version = re.search(
                 r'^__version__\s*=\s*[\'"]([^\'"]*)[\'"]',
                 f.read(), re.MULTILINE).group(1)
-    except IOError as e:
-        if e.errno == errno.ENOENT:
+    except IOError as exc:
+        if exc.errno == errno.ENOENT:
             pass
         else:
             raise
@@ -137,16 +136,12 @@ def module_version(moduledir=MODULEDIR):
 
     # use versioning scheme similar to setuptools_scm for untagged versions
     git_version = get_git_version(REPODIR)
-    if git_version:
-        tag = git_version['tag']
+    if git_version is not None:
+        tag = git_version.tag
         if tag is None:
-            commits = git_version['commits']
-            rev = git_version['rev'][:7]
-            date = datetime.strptime(git_version['date'], '%a, %d %b %Y %H:%M:%S %z')
-            date = datetime.strftime(date, '%Y%m%d')
-            if commits is not None:
+            if (commits := git_version.commits) is not None:
                 version += f'.dev{commits}'
-            version += f'+g{rev}.d{date}'
+            version += f'+g{git_version.short_revision}.d{git_version.date:%Y%m%d}'
         elif tag != version:
             raise DistutilsError(
                 f'unmatched git tag {tag!r} and {MODULE_NAME} version {version!r}')
@@ -164,7 +159,7 @@ def generate_verinfo(target_dir):
     path = os.path.join(target_dir, '_verinfo.py')
     log.info(f'generating version info: {path}')
     with open(path, 'w') as f:
-        f.write('version_info=%r' % (data,))
+        f.write('version_info=%r' % (data._asdict(),))
     return path
 
 
@@ -174,8 +169,8 @@ def readme(topdir=REPODIR):
         try:
             with open(os.path.join(topdir, doc), encoding='utf-8') as f:
                 return f.read()
-        except IOError as e:
-            if e.errno == errno.ENOENT:
+        except IOError as exc:
+            if exc.errno == errno.ENOENT:
                 pass
             else:
                 raise
diff --git a/src/snakeoil/version.py b/src/snakeoil/version.py
index 7b9a5007..0e88dbf1 100644
--- a/src/snakeoil/version.py
+++ b/src/snakeoil/version.py
@@ -3,12 +3,14 @@
 import errno
 import os
 import subprocess
+from datetime import datetime
 from importlib import import_module
+from typing import NamedTuple, Optional
 
 _ver = None
 
 
-def get_version(project, repo_file, api_version=None):
+def get_version(project, repo_file, api_version=None) -> str:
     """Determine a project's version information.
 
     Standardized version retrieval for git-based projects. In summary, if the
@@ -40,74 +42,82 @@ def get_version(project, repo_file, api_version=None):
             version_info = get_git_version(path)
 
         if version_info is None:
-            s = ''
-        elif version_info['tag'] == api_version:
-            s = f" -- released {version_info['date']}"
+            suffix = ''
+        elif version_info.tag == api_version:
+            suffix = f' -- released {version_info.date_rfc2822}'
         else:
-            rev = version_info['rev'][:7]
-            date = version_info['date']
-            commits = version_info.get('commits', None)
-            commits = f'-{commits}' if commits is not None else ''
-            s = f'{commits}-g{rev} -- {date}'
+            rev = version_info.short_revision
+            date = version_info.date_rfc2822
+            commits = f'-{version_info.commits}' if version_info.commits is not None else ''
+            suffix = f'{commits}-g{rev} -- {date}'
 
-        _ver = f'{project} {api_version}{s}'
+        _ver = f'{project} {api_version}{suffix}'
     return _ver
 
 
-def _run_git(path, cmd):
+def _run_git(path: str, *cmd: str):
     env = dict(os.environ)
     for key in env.copy(): # pragma: no cover
         if key.startswith("LC_"):
             del env[key]
     env["LC_CTYPE"] = "C"
     env["LC_ALL"] = "C"
+    r = subprocess.Popen(('git', ) + cmd, env=env, cwd=path,
+        stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
 
-    r = subprocess.Popen(
-        ['git'] + list(cmd), stdout=subprocess.PIPE, env=env,
-        stderr=subprocess.DEVNULL, cwd=path)
-
-    stdout = r.communicate()[0]
+    stdout, _ = r.communicate()
     return stdout, r.returncode
 
 
-def get_git_version(path):
+class GitVersion(NamedTuple):
+    revision: str
+    date: datetime
+    tag: Optional[str] = None
+    commits: Optional[int] = None
+
+    @property
+    def date_rfc2822(self):
+        return self.date.strftime('%a, %d %b %Y %H:%M:%S %z')
+
+    @property
+    def short_revision(self):
+        return self.revision[:7]
+
+
+def get_git_version(path: str) -> Optional[GitVersion]:
     """Return git related revision data."""
     path = os.path.abspath(path)
     try:
-        stdout, ret = _run_git(path, ["log", "--format=%H\n%aD", "HEAD^..HEAD"])
-
+        stdout, ret = _run_git(path, "log", "--format=%H\n%aI", "HEAD^..HEAD")
         if ret != 0:
             return None
 
-        data = stdout.decode().splitlines()
-        tag = _get_git_tag(path, data[0])
+        revision, date = stdout.decode().splitlines()
+        tag = _get_git_tag(path, revision)
 
         # get number of commits since most recent tag
-        stdout, ret = _run_git(path, ['describe', '--tags', '--abbrev=0'])
-        prev_tag = None
+        stdout, ret = _run_git(path, 'describe', '--tags', '--abbrev=0')
         commits = None
         if ret == 0:
             prev_tag = stdout.decode().strip()
-            stgbdout, ret = _run_git(
-                path, ['log', '--oneline', f'{prev_tag}..HEAD'])
+            stdout, ret = _run_git(
+                path, 'log', '--oneline', f'{prev_tag}..HEAD')
             if ret == 0:
                 commits = len(stdout.decode().splitlines())
 
-        return {
-            'rev': data[0],
-            'date': data[1],
-            'tag': tag,
-            'commits': commits,
-        }
-    except EnvironmentError as e:
+        return GitVersion(
+            revision=revision, date=datetime.fromisoformat(date),
+            tag=tag, commits=commits,
+        )
+    except EnvironmentError as exc:
         # ENOENT is thrown when the git binary can't be found.
-        if e.errno != errno.ENOENT:
+        if exc.errno != errno.ENOENT:
             raise
         return None
 
 
 def _get_git_tag(path, rev):
-    stdout, _ = _run_git(path, ['name-rev', '--tag', rev])
+    stdout, _ = _run_git(path, 'name-rev', '--tag', rev)
     tag = stdout.decode().split()
     if len(tag) != 2:
         return None
diff --git a/tests/test_version.py b/tests/test_version.py
index 7dad73e4..841b9917 100644
--- a/tests/test_version.py
+++ b/tests/test_version.py
@@ -1,9 +1,11 @@
+from datetime import datetime
 import errno
 from importlib import reload
 from unittest import mock
 
 import pytest
 from snakeoil import __version__, version
+from snakeoil.version import GitVersion
 
 
 class TestVersion:
@@ -27,31 +29,30 @@ class TestVersion:
         with mock.patch('snakeoil.version.import_module') as import_module, \
                 mock.patch('snakeoil.version.get_git_version') as get_git_version:
             import_module.side_effect = ImportError
-            verinfo = {
-                'rev': '1ff76b021d208f7df38ac524537b6419404f1c64',
-                'date': 'Mon Sep 25 13:50:24 2017 -0400',
-                'tag': None
-            }
-            get_git_version.return_value = verinfo
+            ver_info = GitVersion(
+                revision='1ff76b021d208f7df38ac524537b6419404f1c64',
+                date=datetime.fromisoformat('2017-09-25T13:50:24-04:00'),
+            )
+            get_git_version.return_value = ver_info
 
             result = version.get_version('snakeoil', __file__, __version__)
-            assert result == f"snakeoil {__version__}-g{verinfo['rev'][:7]} -- {verinfo['date']}"
+            assert result == f"snakeoil {__version__}-g1ff76b0 -- {ver_info.date_rfc2822}"
 
     def test_get_version_git_release(self):
-        verinfo = {
-            'rev': 'ab38751890efa8be96b7f95938d6b868b769bab6',
-            'date': 'Thu Sep 21 15:57:38 2017 -0400',
-            'tag': '2.3.4',
-        }
+        ver_info = GitVersion(
+            revision='ab38751890efa8be96b7f95938d6b868b769bab6',
+            date=datetime.fromisoformat('2017-09-21T15:57:38-04:00'),
+            tag='2.3.4',
+        )
 
         # fake snakeoil._verinfo module object
         class Verinfo:
-            version_info = verinfo
+            version_info = ver_info
 
         with mock.patch('snakeoil.version.import_module') as import_module:
             import_module.return_value = Verinfo()
-            result = version.get_version('snakeoil', __file__, verinfo['tag'])
-            assert result == f"snakeoil {verinfo['tag']} -- released {verinfo['date']}"
+            result = version.get_version('snakeoil', __file__, ver_info.tag)
+            assert result == f"snakeoil {ver_info.tag} -- released {ver_info.date_rfc2822}"
 
     def test_get_version_no_git_version(self):
         with mock.patch('snakeoil.version.import_module') as import_module, \
@@ -74,6 +75,20 @@ class TestVersion:
 
 class TestGitVersion:
 
+    def test_date_rfc2822(self):
+        ver = GitVersion(
+            revision='1ff76b021d208f7df38ac524537b6419404f1c64',
+            date=datetime.fromisoformat('2017-09-25T13:50:24-04:00'),
+        )
+        assert ver.date_rfc2822 == 'Mon, 25 Sep 2017 13:50:24 -0400'
+
+    def test_short_revision(self):
+        ver = GitVersion(
+            revision='1ff76b021d208f7df38ac524537b6419404f1c64',
+            date=datetime.now(),
+        )
+        assert ver.short_revision == '1ff76b0'
+
     def test_get_git_version_not_available(self):
         with mock.patch('snakeoil.version._run_git') as run_git:
             run_git.side_effect = EnvironmentError(errno.ENOENT, 'git not found')
@@ -97,31 +112,26 @@ class TestGitVersion:
         with mock.patch('snakeoil.version._run_git') as run_git:
             # dev version
             run_git.return_value = (
-                b'1ff76b021d208f7df38ac524537b6419404f1c64\nMon Sep 25 13:50:24 2017 -0400', 0)
-            result = version.get_git_version('nonexistent')
-            expected = {
-                'rev': '1ff76b021d208f7df38ac524537b6419404f1c64',
-                'date': 'Mon Sep 25 13:50:24 2017 -0400',
-                'tag': None,
-                'commits': 2,
-            }
-            assert result == expected
+                b'1ff76b021d208f7df38ac524537b6419404f1c64\n2017-09-21T15:57:38-04:00', 0)
+            assert version.get_git_version('nonexistent') == GitVersion(
+                revision='1ff76b021d208f7df38ac524537b6419404f1c64',
+                date=datetime.fromisoformat('2017-09-21T15:57:38-04:00'),
+                commits=2,
+            )
 
     def test_get_git_version_good_tag(self):
         with mock.patch('snakeoil.version._run_git') as run_git, \
                 mock.patch('snakeoil.version._get_git_tag') as get_git_tag:
             # tagged, release version
             run_git.return_value = (
-                b'1ff76b021d208f7df38ac524537b6419404f1c64\nMon Sep 25 13:50:24 2017 -0400', 0)
+                b'1ff76b021d208f7df38ac524537b6419404f1c64\n2017-09-21T15:57:38-04:00', 0)
             get_git_tag.return_value = '1.1.1'
-            result = version.get_git_version('nonexistent')
-            expected = {
-                'rev': '1ff76b021d208f7df38ac524537b6419404f1c64',
-                'date': 'Mon Sep 25 13:50:24 2017 -0400',
-                'tag': '1.1.1',
-                'commits': 2,
-            }
-            assert result == expected
+            assert version.get_git_version('nonexistent') == GitVersion(
+                revision='1ff76b021d208f7df38ac524537b6419404f1c64',
+                date=datetime.fromisoformat('2017-09-21T15:57:38-04:00'),
+                tag='1.1.1',
+                commits=2,
+            )
 
     def test_get_git_tag_bad_output(self):
         with mock.patch('snakeoil.version._run_git') as run_git: