launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/lpbuildbot/git-poller into lp:lpbuildbot.

Commit message:
Add a GitPoller implementation.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lpbuildbot/git-poller/+merge/344977

This is a bit different from buildbot's version, in that we're expecting a merge workflow (at least at the moment) and so only want to consider left-hand parents rather than every merged revision for the purpose of things like AggregatingScheduler.  It still involves a fair bit less custom code than BzrPoller though.

I haven't yet plumbed this into master.cfg; that can come when we're ready.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/lpbuildbot/git-poller into lp:lpbuildbot.

=== modified file 'bzrbuildbot/poller.py'
--- bzrbuildbot/poller.py	2018-05-02 16:52:29 +0000
+++ bzrbuildbot/poller.py	2018-05-02 19:41:26 +0000
@@ -1,19 +1,27 @@
-import bzrlib.branch
-import twisted.python.log
-import twisted.internet.defer
-import twisted.internet.reactor
-import twisted.internet.task
-import twisted.internet.threads
-
-import buildbot.util
-import buildbot.changes.base
-import buildbot.changes.changes
+import os
+
+from bzrlib.branch import Branch
+from twisted.internet import (
+    defer,
+    reactor,
+    task,
+    threads,
+    )
+from twisted.internet.utils import getProcessOutput
+from twisted.python import log
+
+from buildbot.changes.base import ChangeSource
+from buildbot.changes.changes import Change
+from buildbot.changes.gitpoller import GitPoller as _GitPoller
+from buildbot.util import (
+    ComparableMixin,
+    epoch2datetime,
+    )
 
 import bzrbuildbot.change
 
 
-class BzrPoller(buildbot.changes.base.ChangeSource,
-                buildbot.util.ComparableMixin):
+class BzrPoller(ChangeSource, ComparableMixin):
 
     compare_attrs = ['urls']
 
@@ -30,7 +38,7 @@
             self.last_revisions[url] = None
         self.polling = False
         self.poll_interval = poll_interval
-        self.loop = twisted.internet.task.LoopingCall(self.poll)
+        self.loop = task.LoopingCall(self.poll)
         self.blame_merge_author = blame_merge_author
         self._objectids = {}
 
@@ -38,28 +46,27 @@
     def urls(self):
         return self.last_revisions.keys()
 
-    @twisted.internet.defer.inlineCallbacks
+    @defer.inlineCallbacks
     def getState(self, url, *args, **kwargs):
         if self._objectids.get(url) is None:
             self._objectids[url] = yield self.master.db.state.getObjectId(
                 url, self.__class__.__name__)
         state = yield self.master.db.state.getState(
             self._objectids[url], *args, **kwargs)
-        twisted.internet.defer.returnValue(state)
+        defer.returnValue(state)
 
-    @twisted.internet.defer.inlineCallbacks
+    @defer.inlineCallbacks
     def setState(self, url, key, value):
         if self._objectids.get(url) is None:
             self._objectids[url] = yield self.master.db.state.getObjectId(
                 url, self.__class__.__name__)
         yield self.master.db.state.setState(self._objectids[url], key, value)
 
-    @twisted.internet.defer.inlineCallbacks
+    @defer.inlineCallbacks
     def startService(self):
-        twisted.python.log.msg("BzrPoller%r starting" % self.urls)
-        buildbot.changes.base.ChangeSource.startService(self)
-        twisted.internet.reactor.callWhenRunning(
-            self.loop.start, self.poll_interval)
+        log.msg("BzrPoller%r starting" % self.urls)
+        ChangeSource.startService(self)
+        reactor.callWhenRunning(self.loop.start, self.poll_interval)
         missing = len([v for v in self.last_revisions.values() if v is None])
         if hasattr(self, 'master') and hasattr(self.master, 'db'):
             for url in self.urls:
@@ -75,15 +82,15 @@
         self.polling = False
 
     def stopService(self):
-        twisted.python.log.msg("BzrPoller%r shutting down" % self.urls)
+        log.msg("BzrPoller%r shutting down" % self.urls)
         if self.loop.running:
             self.loop.stop()
-        return buildbot.changes.base.ChangeSource.stopService(self)
+        return ChangeSource.stopService(self)
 
     def describe(self):
         return "BzrPoller watching %r" % self.urls
 
-    @twisted.internet.defer.inlineCallbacks
+    @defer.inlineCallbacks
     def poll(self):
         if self.polling: # this is called in a loop, and the loop might
             # conceivably overlap.
@@ -97,30 +104,28 @@
         finally:
             self.polling = False
 
-    @twisted.internet.defer.inlineCallbacks
+    @defer.inlineCallbacks
     def _poll(self, url):
         # On a big tree, even individual elements of the bzr commands can
         # take awhile.  So we just push the bzr work off to a thread.
         try:
-            changes = yield twisted.internet.threads.deferToThread(
-                self.getRawChanges, url)
+            changes = yield threads.deferToThread(self.getRawChanges, url)
         except (SystemExit, KeyboardInterrupt):
             raise
         except:
             # we'll try again next poll.  Meanwhile, let's report.
-            twisted.python.log.err()
+            log.err()
         else:
             for change in changes:
                 change['branch'] = url
-                yield self.addChange(
-                    buildbot.changes.changes.Change(**change))
+                yield self.addChange(Change(**change))
                 self.last_revisions[url] = change['revision']
             if hasattr(self, 'master') and hasattr(self.master, 'db'):
                 yield self.setState(
                     url, 'last_revisions', self.last_revisions[url])
 
     def getRawChanges(self, url):
-        branch = bzrlib.branch.Branch.open_containing(url)[0]
+        branch = Branch.open_containing(url)[0]
         try:
             changes = []
             change = bzrbuildbot.change.generate(
@@ -146,7 +151,6 @@
             if hasattr(self.parent, 'addChange'):
                 return self.parent.addChange(change)
             else:
-                from buildbot.util import epoch2datetime
                 return self.master.addChange(
                     author=change.who,
                     revision=change.revision,
@@ -155,5 +159,83 @@
                     when_timestamp=epoch2datetime(change.when),
                     branch=change.branch,
                     src='bzr')
-        twisted.internet.task.deferLater(
-            twisted.internet.reactor, 0, _add_change)
+        task.deferLater(reactor, 0, _add_change)
+
+
+# Unfortunately we have to clone-and-hack quite a lot here, but
+# inlineCallbacks makes it slightly less painful.
+class GitPoller(_GitPoller):
+    """A variant of buildbot's GitPoller that only looks at left-hand parents.
+
+    We don't care about every revision in a branch that gets merged, only
+    the merge itself.
+    """
+
+    @defer.inlineCallbacks
+    def _get_commit_files(self, rev):
+        args = [
+            'log', rev, '--name-only', '%s~..%s' % (rev, rev), r'--format=%n']
+        git_output = yield getProcessOutput(
+            self.gitbin, args, path=self.workdir,
+            env={'PATH': os.environ['PATH']}, errortoo=False)
+        fileList = sorted(set(git_output.split()))
+        defer.returnValue(fileList)
+
+    @defer.inlineCallbacks
+    def _get_commit_name(self, rev):
+        args = ['log', rev, '--no-walk', r'--format=%aN <%aE>']
+        git_output = yield getProcessOutput(
+            self.gitbin, args, path=self.workdir,
+            env={'PATH': os.environ['PATH']}, errortoo=False)
+        stripped_output = git_output.strip().decode(self.encoding)
+        if len(stripped_output) == 0:
+            raise EnvironmentError('could not get commit name for rev')
+        defer.returnValue(stripped_output)
+
+    @defer.inlineCallbacks
+    def _process_changes(self, unused_output):
+        revListArgs = [
+            'log', '--first-parent',
+            '%s..origin/%s' % (self.branch, self.branch), r'--format=%H']
+        self.changeCount = 0
+        results = yield getProcessOutput(
+            self.gitbin, revListArgs, path=self.workdir,
+            env={'PATH': os.environ['PATH']}, errortoo=False)
+
+        # process oldest change first
+        revList = results.split()
+        if not revList:
+            return
+
+        revList.reverse()
+        self.changeCount = len(revList)
+
+        log.msg('gitpoller: processing %d changes: %s in "%s"'
+                % (self.changeCount, revList, self.workdir))
+
+        for rev in revList:
+            results = yield defer.DeferredList([
+                self._get_commit_timestamp(rev),
+                self._get_commit_name(rev),
+                self._get_commit_files(rev),
+                self._get_commit_comments(rev),
+            ], consumeErrors=True)
+
+            # check for failures
+            failures = [ r[1] for r in results if not r[0] ]
+            if failures:
+                # just fail on the first error; they're probably all related!
+                raise failures[0]
+
+            timestamp, name, files, comments = [ r[1] for r in results ]
+            yield self.master.addChange(
+                author=name,
+                revision=rev,
+                files=files,
+                comments=comments,
+                when_timestamp=epoch2datetime(timestamp),
+                branch=self.branch,
+                category=self.category,
+                project=self.project,
+                repository=self.repourl,
+                src='git')