Restore commit logs for Git/GitHub projects

codespeed/commits/git.py

@@ -42,6 +42,34 @@ def updaterepo(project, update=True):
             return [{'error': False}]
 
 
+def get_commit_date(project, commit_id):
+    """Author date of `commit_id` as a naive UTC datetime, or None."""
+    updaterepo(project, update=False)
+    fmt = '%ct' if getattr(settings, 'GIT_USE_COMMIT_DATE', False) else '%at'
+    cmd = ['git', 'log', '-1', '--format=' + fmt, commit_id]
+    p, stdout, stderr = execute_command(cmd, project.working_copy)
+    if p.returncode != 0 or not stdout.strip():
+        logger.warning("%s returned %s: %s", " ".join(cmd), p.returncode, stderr)
+        return None
+    return datetime.datetime.utcfromtimestamp(int(stdout.strip()))
+
+
+class Git:
+    """Git backend with the same interface as commits.mercurial.Mercurial."""
+
+    def __init__(self, project):
+        self.project = project
+
+    def update_repo(self, update=True):
+        return updaterepo(self.project, update=update)
+
+    def get_logs(self, endrev, startrev):
+        return getlogs(endrev, startrev)
+
+    def get_commit_date(self, commit_id):
+        return get_commit_date(self.project, commit_id)
+
+
 def getlogs(endrev, startrev):
     updaterepo(endrev.branch.project, update=False)
 

codespeed/commits/github.py

@@ -126,6 +126,16 @@ def retrieve_revision(commit_id, username, project, revision=None):
             'tag':          tag}
 
 
+def get_commit_date(project, commit_id):
+    """Committer date of `commit_id` from the GitHub API, or None."""
+    m = GITHUB_URL_RE.match(project.repo_path.rstrip('/'))
+    if not m:
+        raise CommitLogError(
+            "Unable to parse Github URL %s" % project.repo_path)
+    data = retrieve_revision(commit_id, m.group("username"), m.group("project"))
+    return data['date'] if data else None
+
+
 def getlogs(endrev, startrev):
     if endrev != startrev:
         revisions = endrev.branch.revisions.filter(

codespeed/commits/logs.py

@@ -4,21 +4,54 @@
 import datetime
 import logging
 
+from .exceptions import CommitLogError
+
 logger = logging.getLogger(__name__)
 
 
+class FunctionBackend:
+    """Adapter giving the function-based backends (GitHub, Subversion) the
+    same interface as the Mercurial and Git classes."""
+
+    def __init__(self, project, module):
+        self.project = project
+        self.module = module
+
+    def update_repo(self, update=True):
+        # These backends talk to a remote service; there is no clone to
+        # create, so only "update" means anything.
+        if update:
+            return self.module.updaterepo(self.project)
+
+    def get_logs(self, endrev, startrev):
+        return self.module.getlogs(endrev, startrev)
+
+    def get_commit_date(self, commit_id):
+        lookup = getattr(self.module, 'get_commit_date', None)
+        if lookup is None:
+            return None  # not supported; callers fall back to today's date
+        return lookup(self.project, commit_id)
+
+
 def get_scm(project):
-    if project.repo_type == project.SUBVERSION:
-        from .subversion import getlogs, updaterepo
-    elif project.repo_type == project.MERCURIAL:
+    """Return a backend object for the project, or None if logs are off.
+
+    Every backend has update_repo(update), get_logs(endrev, startrev) and
+    get_commit_date(commit_id).
+    """
+    if project.repo_type == project.MERCURIAL:
         from .mercurial import Mercurial
         return Mercurial(project)
     elif project.repo_type == project.GIT:
-        from .git import getlogs, updaterepo
+        from .git import Git
+        return Git(project)
     elif project.repo_type == project.GITHUB:
-        from .github import getlogs, updaterepo  # noqa: F401
-    else:
-        return None
+        from . import github
+        return FunctionBackend(project, github)
+    elif project.repo_type == project.SUBVERSION:
+        from . import subversion
+        return FunctionBackend(project, subversion)
+    return None
 
 
 def get_logs(rev, startrev, update=False):
@@ -33,7 +66,7 @@ def get_logs(rev, startrev, update=False):
         return logs
 
     if update:
-        scm.update_repo(project)
+        scm.update_repo()
 
     logs = scm.get_logs(rev, startrev)
 
@@ -49,7 +82,13 @@ def _get_commit_date(project, commit_id):
     if scm is None:
         return None
 
-    return scm.get_commit_date(commit_id)
+    try:
+        return scm.get_commit_date(commit_id)
+    except CommitLogError as e:
+        # A missing clone or unknown commit must not break result submission
+        logger.warning("Could not get the date of commit %r of project %s: %s",
+                       commit_id, project, e)
+        return None
 
 
 def get_commit_date(project, commit_id):

codespeed/commits/tests/test_git.py

@@ -2,7 +2,9 @@
 from django.test import TestCase, override_settings
 from unittest.mock import Mock, patch
 
-from codespeed.commits.git import getlogs
+from codespeed import commits
+from codespeed.commits.git import Git, getlogs
+from codespeed.commits.logs import FunctionBackend, get_scm
 from codespeed.models import Project, Revision, Branch, Environment
 
 
@@ -58,3 +60,77 @@ def side_effect(cmd, *args, **kwargs):
             'tag': 'tag',
         }
         self.assertEqual([expected], logs)
+
+    @patch("codespeed.commits.git.Popen")
+    def test_logs_through_dispatcher(self, popen):
+        """commits.get_logs must reach the git backend for a Git project."""
+        ret = Mock()
+        ret.returncode = 0
+        ret.communicate.return_value = (
+            b"id\x00long_id\x001583489681\x00author\x00email\x00msg\x00\x1e", b'')
+        popen.return_value = ret
+
+        logs = commits.get_logs(self.revision, self.revision)
+
+        self.assertEqual(len(logs), 1)
+        self.assertEqual(logs[0]['commitid'], 'long_id')
+
+    @patch("codespeed.commits.git.Popen")
+    def test_get_commit_date(self, popen):
+        ret = Mock()
+        ret.returncode = 0
+        ret.communicate.return_value = (b"1583489681\n", b'')
+        popen.return_value = ret
+
+        date = commits.get_commit_date(self.project, 'long_id')
+
+        self.assertEqual(date, datetime(2020, 3, 6, 10, 14, 41))
+        cmd = popen.call_args_list[-1][0][0]
+        self.assertEqual(cmd[:3], ['git', 'log', '-1'])
+        self.assertEqual(cmd[-1], 'long_id')
+
+    @patch("codespeed.commits.git.Popen")
+    def test_get_commit_date_unknown_commit_falls_back(self, popen):
+        ret = Mock()
+        ret.returncode = 128
+        ret.communicate.return_value = (b'', b'fatal: bad object')
+        popen.return_value = ret
+
+        date = commits.get_commit_date(self.project, 'nope')
+
+        # Unknown commit: today rather than an exception, as before
+        self.assertEqual(date.date(), datetime.today().date())
+
+
+class GetScmTest(TestCase):
+    def make(self, repo_type, path='https://github.com/python/cpython'):
+        return Project.objects.create(name='p-%s' % repo_type, repo_path=path,
+                                      repo_type=repo_type)
+
+    def test_git(self):
+        self.assertIsInstance(get_scm(self.make(Project.GIT)), Git)
+
+    def test_github_and_subversion_get_an_adapter(self):
+        for repo_type in (Project.GITHUB, Project.SUBVERSION):
+            self.assertIsInstance(get_scm(self.make(repo_type)), FunctionBackend)
+
+    def test_subversion_has_no_commit_date(self):
+        self.assertIsNone(get_scm(self.make(Project.SUBVERSION)).get_commit_date('1'))
+
+    @patch("codespeed.commits.github.fetch_json")
+    def test_github_commit_date_from_api(self, fetch_json):
+        def fake(url):
+            if url.endswith('/git/refs/tags'):
+                return []
+            return {'sha': 'abc', 'parents': [], 'message': 'm',
+                    'committer': {'date': '2020-03-06T10:14:41Z'},
+                    'author': {'name': 'a', 'email': 'a@example.com'}}
+        fetch_json.side_effect = fake
+        date = commits.get_commit_date(self.make(Project.GITHUB), 'abc')
+        self.assertEqual(date.isoformat(), '2020-03-06T10:14:41+00:00')
+        self.assertIn('/repos/python/cpython/git/commits/abc',
+                      fetch_json.call_args_list[0][0][0])
+
+    def test_no_logs(self):
+        self.assertIsNone(get_scm(self.make(Project.NO_LOGS)))
+        self.assertEqual(commits.get_logs.__name__, 'get_logs')