Merge ~nacc/usd-importer:changelog-parsing into usd-importer:master

[Nish Aravamudan]

Supersedes: https://code.launchpad.net/~racb/usd-importer/+git/usd-importer/+merge/324476

Reworked to include tests.

[Robie Basak (racb)]

Please add the assert statements comparing against dpkg-parsechangelog output as discussed in HO.

Does acc1a9e817d00c8733177d94801aafdf688b55fa have any implications for the dpkg-parsechangelog method results? Could it cause a change in behaviour against the previous method?

You mentioned that a commit removes whitespace unintentionally.

review: Approve

diff --git a/gitubuntu/__main__.py b/gitubuntu/__main__.py
index b2ba8d2..ba4a435 100644
--- a/gitubuntu/__main__.py
+++ b/gitubuntu/__main__.py
@@ -182,7 +182,10 @@ def main():
                 args.retry_backoffs = [2 ** i for i in range(args.retries)]
 
         try:
-            run(['git', 'config', 'gitubuntu.lpuser'], quiet=True)
+            run(
+                ['git', 'config', 'gitubuntu.lpuser'],
+                verbose_on_failure=False,
+            )
         except CalledProcessError:
             if isatty(sys.stdin.fileno()):
                 user = input("gitubuntu.lpuser is not set. What is your "
diff --git a/gitubuntu/clone.py b/gitubuntu/clone.py
index 76de08f..a1bc8cb 100644
--- a/gitubuntu/clone.py
+++ b/gitubuntu/clone.py
@@ -5,7 +5,10 @@ import re
 import shutil
 from subprocess import CalledProcessError
 import sys
-from gitubuntu.git_repository import GitUbuntuRepository
+from gitubuntu.git_repository import (
+    GitUbuntuRepository,
+    GitUbuntuRepositoryFetchError,
+)
 from gitubuntu.run import decode_binary, run
 
 try:
@@ -93,8 +96,8 @@ Example:
 
         try:
             local_repo.add_base_remotes(args.package)
-            local_repo.fetch_base_remotes(must_exist=True)
-        except SystemExit:
+            local_repo.fetch_base_remotes(verbose=True)
+        except GitUbuntuRepositoryFetchError:
             logging.error("Unable to find an imported repository for %s. "
                 "Please request an import by e-mailing "
                 "[e-mail].",
@@ -105,12 +108,12 @@ Example:
 
         try:
             local_repo.add_lpuser_remote(pkgname=args.package)
-            local_repo.fetch_lpuser_remote(must_exist=False)
+            local_repo.fetch_lpuser_remote(verbose=True)
 
             logging.debug("added remote '%s' -> %s", local_repo.lp_user,
                 local_repo.raw_repo.remotes[local_repo.lp_user].url
             )
-        except:
+        except GitUbuntuRepositoryFetchError:
             pass
 
         try:
diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
index b505da3..6d3eb00 100644
--- a/gitubuntu/git_repository.py
+++ b/gitubuntu/git_repository.py
@@ -3,11 +3,11 @@
 
 import collections
 from copy import copy
-from enum import Enum, unique
-import functools
+from functools import lru_cache
 import itertools
 import logging
 import os
+import posixpath
 import re
 import shutil
 import stat
@@ -17,45 +17,312 @@ import tempfile
 import time
 from gitubuntu.run import run, runq, decode_binary
 try:
+    pkg = 'python3-debian'
+    import debian.changelog
     pkg = 'python3-pygit2'
     import pygit2
+    pkg = 'python3-pytest'
+    import pytest
 except ImportError:
     logging.error('Is %s installed?', pkg)
     sys.exit(1)
 
 
-def memoize(obj):
-    cache = obj.cache = {}
+def _follow_symlinks_to_blob(repo, top_tree_object, search_path,
+    _rel_tree=None, _rel_path=''
+):
+    '''Recursively follow a path down a tree, following symlinks, to find blob
+
+    repo: pygit2.Repository object
+    top_tree: pygit2.Tree object of the top of the tree structure
+    search_path: '/'-separated path string of blob to find
+    _rel_tree: (internal) which tree to look further into
+    _rel_path: (internal) the path we are in so far
+    '''
+
+    NORMAL_BLOB_MODES = set([
+        pygit2.GIT_FILEMODE_BLOB,
+        pygit2.GIT_FILEMODE_BLOB_EXECUTABLE,
+    ])
+
+    _rel_tree = _rel_tree or top_tree_object
+    head, tail = posixpath.split(search_path)
+
+    # A traditional functional split would put a single entry in head with tail
+    # empty, but posixpath.split doesn't necessarily do this. Jiggle it round
+    # to make it appear to have traditional semantics.
+    if not head:
+        head = tail
+        tail = None
+
+    entry = _rel_tree[head]
+    if entry.type == 'tree':
+        return _follow_symlinks_to_blob(
+            repo=repo,
+            top_tree_object=top_tree_object,
+            search_path=tail,
+            _rel_tree=repo.get(entry.id),
+            _rel_path=posixpath.join(_rel_path, head),
+        )
+    elif entry.type == 'blob' and entry.filemode == pygit2.GIT_FILEMODE_LINK:
+        # Found a symlink. Start again from the top with adjustment for symlink
+        # following
+        search_path = posixpath.normpath(
+            posixpath.join(_rel_path, repo.get(entry.id).data, tail)
+        )
+        return _follow_symlinks_to_blob(
+            repo=repo,
+            top_tree_object=top_tree_object,
+            search_path=search_path,
+        )
+    elif entry.type == 'blob' and entry.filemode in NORMAL_BLOB_MODES:
+        return repo.get(entry.id)
+    else:
+        # Found some special entry such as a "gitlink" (submodule entry)
+        raise ValueError(
+            "Found %r filemode %r looking for %r" %
+                (entry, entry.filemode, posixpath.join(_rel_path, search_path))
+        )
 
-    @functools.wraps(obj)
-    def memoizer(*args, **kwargs):
-        key = str(args) + str(kwargs)
-        if key not in cache:
-            cache[key] = obj(*args, **kwargs)
-        else:
-            logging.debug("Cache hit on %s %s = %s", str(args),
-                          str(kwargs),
-                          cache[key][:30].replace("\n", "\\n") + "..")
-        return cache[key]
-    return memoizer
 
+def follow_symlinks_to_blob(repo, treeish_object, path):
+    return _follow_symlinks_to_blob(
+        repo=repo,
+        top_tree_object=treeish_object.peel(pygit2.Tree),
+        search_path=posixpath.normpath(path),
+    )
 
-class GitUbuntuChangelogError(Exception):
+class ChangelogError(Exception):
     pass
 
+class Changelog:
+    '''Representation of a debian/changelog file found inside a git tree-ish
+
+    Uses dpkg-parsechangelog for parsing, but when this fails we fall
+    back to grep/sed-based pattern matching automatically.
+    '''
+    def __init__(self, content_bytes):
+        '''
+        contents: bytes string of file contents
+        '''
+        self._contents = content_bytes
+        self._changelog = debian.changelog.Changelog(self._contents)
+        if not len(self._changelog.versions):
+            # assume bad read, so fall back to shell later
+            self._changelog = None
+
+    @classmethod
+    def from_treeish(cls, repo, treeish_object):
+        '''
+        repo: pygit2.Repository instance
+        treeish_object: pygit2.Object subclass instance (must peel to pygit2.Tree)
+        '''
+        blob = follow_symlinks_to_blob(
+            repo=repo,
+            treeish_object=treeish_object,
+            path='debian/changelog'
+        )
+        return cls(blob.data)
+
+    @classmethod
+    def from_path(cls, path):
+        with open(path, 'rb') as f:
+            return cls(f.read())
+
+    @lru_cache()
+    def _dpkg_parsechangelog(self, parse_params):
+        cp = run(
+            'dpkg-parsechangelog -l- %s' % parse_params,
+            input=self._contents,
+            shell=True,
+            verbose_on_failure=False,
+        )
+        return decode_binary(cp.stdout).strip()
+
+    @lru_cache()
+    def _shell(self, cmd):
+        cp = run(
+            cmd,
+            input=self._contents,
+            shell=True,
+            verbose_on_failure=False,
+        )
+        return decode_binary(cp.stdout).strip()
+
+    @property
+    def _shell_version(self):
+        parse_params = '-n1 -SVersion'
+        shell_cmd = 'grep -m1 \'^\S\' | sed \'s/.*(\(.*\)).*/\\1/\''
+        try:
+            raw_out = self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            raw_out = self._shell(shell_cmd)
+        return None if raw_out == '' else raw_out
+
+    @property
+    def version(self):
+        if self._changelog:
+            try:
+                ret = str(self._changelog.versions[0])
+                if ret != self._shell_version:
+                    raise ChangelogError(
+                        'Old (%s) and new (%s) changelog values do not agree' %
+                        (self._shell_version, ret)
+                    )
+                return ret
+            except IndexError:
+                return None
+        return self._shell_version
+
+    @property
+    def _shell_previous_version(self):
+        parse_params = '-n1 -o1 -SVersion'
+        shell_cmd = 'grep -m1 \'^\S\' | tail -1 | sed \'s/.*(\(.*\)).*/\\1/\''
+        try:
+            raw_out = self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            raw_out = self._shell(shell_cmd)
+        return None if raw_out == '' else raw_out
+
+    @property
+    def previous_version(self):
+        if self._changelog:
+            try:
+                ret = str(self._changelog.versions[1])
+                if ret != self._shell_previous_version:
+                    raise ChangelogError(
+                        'Old (%s) and new (%s) changelog values do not agree' %
+                        (self._shell_previous_version, ret)
+                    )
+                return ret
+            except IndexError:
+                return None
+        return self._shell_previous_version
+
+    @property
+    def _shell_maintainer(self):
+        parse_params = '-SMaintainer'
+        shell_cmd = 'grep -m1 \'^ --\' | sed \'s/ -- \(.*\)  \(.*\)/\\1/\''
+        try:
+            return self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            return self._shell(shell_cmd)
+
+    @property
+    def maintainer(self):
+        if self._changelog:
+            ret = self._changelog.author
+            if ret != self._shell_maintainer:
+                raise ChangelogError(
+                    'Old (%s) and new (%s) changelog values do not agree' %
+                    (self._shell_maintainer, ret)
+                )
+            return ret
+        return self._shell_maintainer
+
+    @property
+    def _shell_date(self):
+        parse_params = '-SDate'
+        shell_cmd = 'grep -m1 \'^ --\' | sed \'s/ -- \(.*\)  \(.*\)/\\2/\''
+        try:
+            return self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            return self._shell(shell_cmd)
+
+    @property
+    def date(self):
+        if self._changelog:
+            ret = self._changelog.date
+            if ret != self._shell_date:
+                raise ChangelogError(
+                    'Old (%s) and new (%s) changelog values do not agree' %
+                    (self._shell_date, ret)
+                )
+            return ret
+        return self._shell_date
+
+    @property
+    def _shell_all_versions(self):
+        parse_params = '--format rfc822 -SVersion --all'
+        shell_cmd = 'grep \'^\S\' | sed \'s/.*(\(.*\)).*/\\1/\''
+        try:
+            version_lines = self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            version_lines = self._shell(shell_cmd)
+        return [
+            v_stripped
+            for v_stripped in (
+                v.strip() for v in version_lines.splitlines()
+            )
+            if v_stripped
+        ]
+
+    @property
+    def all_versions(self):
+        if self._changelog:
+            ret = [str(v) for v in self._changelog.versions]
+            if ret != self._shell_all_versions:
+                raise ChangelogError(
+                    'Old (%s) and new (%s) changelog values do not agree' %
+                    (self._shell_all_versions, ret)
+                )
+            return ret
+        return self._shell_all_versions
+
+    @property
+    def _shell_distribution(self):
+        parse_params = '-SDistribution'
+        shell_cmd = 'grep -m1 \'^\S\' | sed \'s/.*\ .*\ \(.*\);.*/\\1/\''
+        try:
+            return self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            return self._shell(shell_cmd)
+
+    @property
+    def distribution(self):
+        if self._changelog:
+            ret = self._changelog.package
+            if ret != self._shell_distribution:
+                raise ChangelogError(
+                    'Old (%s) and new (%s) changelog values do not agree' %
+                    (self._shell_srcpkg, ret)
+                )
+            return ret
+        return self._shell_distribution
+
+    @property
+    def _shell_srcpkg(self):
+        parse_params = '-SSource'
+        shell_cmd = 'grep -m1 \'^\S\' | sed \'s/\(.*\)\ .*\ .*;.*/\\1/\''
+        try:
+            return self._dpkg_parsechangelog(parse_params)
+        except CalledProcessError:
+            return self._shell(shell_cmd)
+
+    @property
+    def srcpkg(self):
+        if self._changelog:
+            ret = self._changelog.package
+            if ret != self._shell_srcpkg:
+                raise ChangelogError(
+                    'Old (%s) and new (%s) changelog values do not agree' %
+                    (self._shell_srcpkg, ret)
+                )
+            return ret
+        return self._shell_srcpkg
+
[e-mail]('changelog_path, expected', [
+    ('tests/changelogs/test_versions_1', ['1.0', None]),
+    ('tests/changelogs/test_versions_2', ['2.0', '1.0']),
+    ('tests/changelogs/test_versions_3', ['4.0', '3.0']),
+])
+def test_changelog_versions(changelog_path, expected):
+    test_changelog = Changelog.from_path(changelog_path)
+    assert [test_changelog.version, test_changelog.previous_version] == expected
+
+class GitUbuntuChangelogError(Exception):
+    pass
 
-@unique
-class ChangelogField(Enum):
-    """Trivial enum for specifying fields to extract from
-    debian/changelog
-    """
-    version = 1
-    previous_version = 2
-    maintainer = 3
-    date = 4
-    all_versions = 5
-    distribution = 6
-    srcpkg = 7
 
 def git_dep14_tag(version):
     """Munge a version string according to taken from
@@ -87,6 +354,11 @@ def upstream_tag(version, namespace):
 def orphan_tag(version, namespace):
     return '%s/orphan/%s' % (namespace, git_dep14_tag(version))
 
+
+class GitUbuntuRepositoryFetchError(Exception):
+    pass
+
+
 class GitUbuntuRepository:
     """A class for interacting with an importer git repository
 
@@ -131,8 +403,10 @@ class GitUbuntuRepository:
             self._lp_user = lp_user
         else:
             try:
-                cp = self.git_run(['config', 'gitubuntu.lpuser'],
-                                  quiet=True)
+                cp = self.git_run(
+                    ['config', 'gitubuntu.lpuser'],
+                    verbose_on_failure=False,
+                )
                 self._lp_user = decode_binary(cp.stdout).strip()
             except CalledProcessError:
                 logging.error("Unable to determine Launchpad user")
@@ -263,46 +537,53 @@ class GitUbuntuRepository:
         # XXX: want a remote alias of 'lpme' -> self.lp_user
         # self.git_run(['config', 'url.%s.insteadof' % self.lp_user, 'lpme'])
 
-    def _fetch_remote(self, remote_name, must_exist):
+    def fetch_remote(self, remote_name, verbose=False):
         try:
             # Does not seem to be working with https
             # self.raw_repo.remotes[remote_name].fetch()
             logging.debug('Fetching remote %s' % remote_name)
-            self.git_run(['fetch', remote_name], quiet=not must_exist)
+            kwargs = {}
+            kwargs['verbose_on_failure'] = True
+            if verbose:
+                # If we are redirecting stdout/stderr to the console, we
+                # do not need to have run() also emit it
+                kwargs['verbose_on_failure'] = False
+                kwargs['stdout'] = None
+                kwargs['stderr'] = None
+            self.git_run(['fetch', remote_name], **kwargs)
         except CalledProcessError:
-            if must_exist:
-                logging.error('No objects found in remote %s', remote_name)
-                sys.exit(1)
-            else:
-                logging.debug('No objects found in remote %s', remote_name)
-
-    def fetch_remote(self, remote_name, must_exist):
-        self._fetch_remote(remote_name=remote_name,
-                           must_exist=must_exist)
+            raise GitUbuntuRepositoryFetchError(
+                'Unable to fetch remote %s' % remote_name
+            )
 
-    def fetch_base_remotes(self, must_exist):
-        self.fetch_remote(remote_name='pkg', must_exist=must_exist)
+    def fetch_base_remotes(self, verbose=False):
+        self.fetch_remote(remote_name='pkg', verbose=verbose)
 
-    def fetch_remote_refspecs(self, remote_name, refspecs, must_exist):
+    def fetch_remote_refspecs(self, remote_name, refspecs, verbose=False):
         try:
             # Does not seem to be working with https
             # self.raw_repo.remotes[remote_name].fetch()
             for refspec in refspecs:
                 logging.debug('Fetching refspec %s from remote %s' %
                              (refspec, remote_name))
-                self.git_run(['fetch', remote_name, refspec],
-                             quiet=not must_exist)
+                kwargs = {}
+                kwargs['verbose_on_failure'] = True
+                if verbose:
+                    # If we are redirecting stdout/stderr to the console, we
+                    # do not need to have run() also emit it
+                    kwargs['verbose_on_failure'] = False
+                    kwargs['stdout'] = None
+                    kwargs['stderr'] = None
+                self.git_run(['fetch', remote_name, refspec], **kwargs)
         except CalledProcessError:
-            logging.debug('No objects found in remote %s', remote_name)
-            if must_exist:
-                raise
+            raise GitUbuntuRepositoryFetchError(
+                'Unable to fetch %s from remote %s' % (refspecs, remote_name)
+            )
 
-    def fetch_lpuser_remote(self, must_exist=False):
+    def fetch_lpuser_remote(self, verbose=False):
         if not self._fetch_proto:
             raise Exception('Cannot fetch using an object without a protocol')
-        self._fetch_remote(remote_name=self.lp_user,
-                           must_exist=must_exist)
-
+        self.fetch_remote(remote_name=self.lp_user, verbose=verbose)
 
     def copy_base_references(self, namespace):
         for ref in self.references:
@@ -418,176 +699,78 @@ class GitUbuntuRepository:
     def garbage_collect(self):
         self.git_run(['gc'])
 
-    def extract_file_from_treeish(self, treeish, filename, outfile=None):
+    def extract_file_from_treeish(self, treeish_string, filename):
         """extract a file from @treeish to a local file
 
         Arguments:
         treeish - SHA1 of treeish
         filename - file to extract from @treeish
-        outfile - name of file to copy @filename contents to, will be overwritten.
-                  If None, will be a NamedTemporaryFile.
-        """
-        if outfile is None:
-            outfile = tempfile.NamedTemporaryFile(mode='w+')
-        else:
-            outfile = open(outfile, mode='w+')
-
-        changelog_file = '%s:debian/changelog' % treeish
 
-        cat_changelog_cmd = (
-            "echo %s | "
-            "git cat-file --batch --follow-symlinks | "
-            "sed -n '1{/^[^ ]* blob/!{p;q1}};${/^$/d};2,$p'"
-            % changelog_file
+        Returns a NamedTemporaryFile that is flushed but not rewound.
+        """
+        blob = follow_symlinks_to_blob(
+            self.raw_repo,
+            treeish_object=self.raw_repo.get(treeish_string),
+            path=filename,
         )
-
-        try:
-            cp = run(cat_changelog_cmd, shell=True, env=self._env,
-                     stdout=outfile)
-        except CalledProcessError:
-            raise GitUbuntuChangelogError('Unable to extract file')
-
-        outfile.seek(0)
-        if '%s missing' % changelog_file in outfile.read():
-            raise GitUbuntuChangelogError('debian/changelog not found in '
-                                          '%s' % treeish)
-
+        outfile = tempfile.NamedTemporaryFile()
+        outfile.write(blob.data)
         outfile.flush()
         return outfile
 
-    @memoize
-    def parse_changelog_field_in_treeish(self, treeish, field):
-        """Parse debian/changelog for specified field, using
-        dpkg-parsechangelog with fallback to shell munging
+    @lru_cache()
+    def get_changelog_from_treeish(self, treeish_string):
+        return Changelog.from_treeish(self.raw_repo, self.raw_repo.get(treeish_string))
 
-        dpkg-parsechangelog is preferentially used, however there have
-        been multiple cases where historical changelogs have not been
-        parseable. Fallback to a relatively dumb shell-based parsing for
-        relevant fields in that case. If that also fails, as a last
-        resort, see if any source patches are applicable.
-
-        Arguments:
-        treeish -- SHA1 of treeish
-        field -- ChangelogField value corresponding to field to extract
+    def get_changelog_versions_from_treeish(self, treeish_string):
+        """Extract current and prior versions from debian/changelog in a
+        given @treeish_string
 
-        Returns:
-        A subprocess.CompletedProcess instance as returned by
-        subprocess.run
+        Returns (None, None) if the treeish supplied is None or if
+        'debian/changelog' does not exist in the treeish.
         """
-        # in each case:
-        # parse_params is what is passed on to dpkg-parsechangelog
-        # shell_cmd is a suitable shell sequence to pipe the changelog
-        # through to achieve the same if dpkg-parsechangelog fails
-        if field is ChangelogField.version:
-            parse_params = '-n1 -SVersion'
-            shell_cmd = 'grep -m1 \'^\S\' | sed \'s/.*(\(.*\)).*/\\1/\''
-        elif field is ChangelogField.previous_version:
-            parse_params = '-n1 -o1 -SVersion'
-            shell_cmd = 'grep -m1 \'^\S\' | tail -1 | sed \'s/.*(\(.*\)).*/\\1/\''
-        elif field is ChangelogField.maintainer:
-            parse_params = '-SMaintainer'
-            shell_cmd = 'grep -m1 \'^ --\' | sed \'s/ -- \(.*\)  \(.*\)/\\1/\''
-        elif field is ChangelogField.date:
-            parse_params = '-SDate'
-            shell_cmd = 'grep -m1 \'^ --\' | sed \'s/ -- \(.*\)  \(.*\)/\\2/\''
-        elif field is ChangelogField.all_versions:
-            parse_params = '--format rfc822 -SVersion --all'
-            shell_cmd = 'grep \'^\S\' | sed \'s/.*(\(.*\)).*/\\1/\''
-        elif field is ChangelogField.distribution:
-            parse_params = '-SDistribution'
-            shell_cmd = 'grep -m1 \'^\S\' | sed \'s/.*\ .*\ \(.*\);.*/\\1/\''
-        elif field is ChangelogField.srcpkg:
-            parse_params = '-SSource'
-            shell_cmd = 'grep -m1 \'^\S\' | sed \'s/\(.*\)\ .*\ .*;.*/\\1/\''
-        else:
-            raise GitUbuntuChangelogError('Unknown changelog field %s' % field)
-
-        # We cannot use "git show" since it does not follow symlinks (LP:
-        # #1661092). Instead, use "git cat-file --batch --follow-symlinks" and
-        # parse the batch output (first line) to ensure that we get a blob
-        # rather than a symlink following failure. If we don't get a blob, then
-        # print what we got and exit 1.
-        changelog_file = '%s:debian/changelog' % treeish
-        cat_changelog_cmd = (
-            "echo %s | "
-            "git cat-file --batch --follow-symlinks | "
-            "sed -n '1{/^[^ ]* blob/!{p;q1}};2,$p'"
-            % changelog_file
-        )
-
         try:
-            cp = run(
-                '%s | dpkg-parsechangelog -l- %s' %
-                (cat_changelog_cmd, parse_params),
-                shell=True, env=self._env, quiet=True)
+            changelog = self.get_changelog_from_treeish(treeish_string)
+        except KeyError:
+            # If 'debian/changelog' does
+            # not exist, then (None, None) is returned. KeyError propagates up
+            # from Changelog's __init__.
+            return None, None
+        try:
+            return changelog.version, changelog.previous_version
         except CalledProcessError:
-            try:
-                cp = run(
-                    '%s | %s' % (cat_changelog_cmd, shell_cmd),
-                    shell=True, env=self._env, quiet=True)
-            except CalledProcessError:
-                raise GitUbuntuChangelogError('Unable to parse changelog')
-
-        out = decode_binary(cp.stdout).strip()
-
-        if '%s missing' % changelog_file in out:
-            raise GitUbuntuChangelogError('debian/changelog not found in '
-                                          '%s' % treeish)
-
-        return out
-
-    def get_changelog_versions_from_treeish(self, treeish):
-        """Extract current and prior versions from debian/changelog in a
-        given treeish
-        """
-        current_version = None
-        last_version = None
-        if treeish is not None:
-            try:
-                current_version = self.parse_changelog_field_in_treeish(
-                         treeish, ChangelogField.version
-                                                                       )
-                last_version = self.parse_changelog_field_in_treeish(
-                         treeish, ChangelogField.previous_version
-                                                                    )
-
-            except:
-                raise GitUbuntuChangelogError('Cannot get changelog versions')
-
-        return (current_version, last_version)
+            raise GitUbuntuChangelogError(
+                'Cannot get changelog versions'
+            )
 
-    def get_changelog_distribution_from_treeish(self, treeish):
+    def get_changelog_distribution_from_treeish(self, treeish_string):
         """Extract targetted distribution from debian/changelog in a
         given treeish
         """
-        distribution = None
-        if treeish is not None:
-            try:
-                distribution = self.parse_changelog_field_in_treeish(
-                         treeish, ChangelogField.distribution
-                                                                    )
-            except:
-                raise GitUbuntuChangelogError(
-                    'Cannot get changelog distribution'
-                )
 
-        return distribution
+        if treeish_string is None:
+            return None
+
+        try:
+            return self.get_changelog_from_treeish(treeish_string).distribution
+        except (KeyError, CalledProcessError):
+            raise GitUbuntuChangelogError(
+                'Cannot get changelog distribution'
+            )
 
-    def get_changelog_srcpkg_from_treeish(self, treeish):
+    def get_changelog_srcpkg_from_treeish(self, treeish_string):
         """Extract srcpkg from debian/changelog in a given treeish
         """
-        srcpkg = None
-        if treeish is not None:
-            try:
-                srcpkg = self.parse_changelog_field_in_treeish(
-                         treeish, ChangelogField.srcpkg
-                                                                    )
-            except:
-                raise GitUbuntuChangelogError(
-                    'Cannot get changelog source package name'
-                )
 
-        return srcpkg
+        if treeish_string is None:
+            return None
+
+        try:
+            return self.get_changelog_from_treeish(treeish).srcpkg
+        except (KeyError, CalledProcessError):
+            raise GitUbuntuChangelogError(
+                'Cannot get changelog source package name'
+            )
 
     def get_heads_and_versions(self, head_prefix, namespace):
         """Extract the last version in debian/changelog of all
@@ -610,12 +793,12 @@ class GitUbuntuRepository:
 
         return versions
 
-    def treeishs_identical(self, treeish1, treeish2):
-        if treeish1 is None or treeish2 is None:
+    def treeishs_identical(self, treeish_string1, treeish_string2):
+        if treeish_string1 is None or treeish_string2 is None:
             return False
-        _treeish1 = self.raw_repo.get(treeish1).peel(pygit2.Tree).id
-        _treeish2 = self.raw_repo.get(treeish2).peel(pygit2.Tree).id
-        return _treeish1 == _treeish2
+        _tree_id1 = self.raw_repo.get(treeish_string1).peel(pygit2.Tree).id
+        _tree_id2 = self.raw_repo.get(treeish_string2).peel(pygit2.Tree).id
+        return _tree_id1 == _tree_id2
 
     def get_head_by_name(self, name):
         try:
@@ -719,12 +902,9 @@ class GitUbuntuRepository:
     def get_commit_authorship(self, ref, spi):
         """Extract last debian/changelog entry's maintainer and date"""
         try:
-            author = self.parse_changelog_field_in_treeish(
-                ref, ChangelogField.maintainer
-                                                          )
-            date = self.parse_changelog_field_in_treeish(
-                ref, ChangelogField.date
-                                                        )
+            changelog = self.get_changelog_from_treeish(ref)
+            author = changelog.maintainer
+            date = changelog.date
         except:
             logging.exception('Cannot get commit authorship for %s' % ref)
             sys.exit(1)
@@ -868,19 +1048,12 @@ class GitUbuntuRepository:
         runq(['git', 'clean', '-f', '-d'], env=self._env)
 
     def get_all_changelog_versions_from_treeish(self, treeish):
+        changelog = self.get_changelog_from_treeish(treeish)
         try:
-            lines = self.parse_changelog_field_in_treeish(
-                     treeish, ChangelogField.all_versions
-                                                         )
+            return changelog.all_versions
         except:
             logging.exception('Cannot get all versions from changelog')
             sys.exit(1)
-        versions = list()
-        for version in lines.splitlines():
-            version = version.strip()
-            if len(version) > 0:
-                versions.append(version)
-        return versions
 
     def annotated_tag(self, tag_name, commitish, force, msg=None):
         try:
@@ -973,7 +1146,7 @@ class GitUbuntuRepository:
         return tree_builder
 
     @classmethod
-    def _add_missing_tree_dirs(cls, repo, top_path, top_tree, _sub_path=''):
+    def _add_missing_tree_dirs(cls, repo, top_path, top_tree_object, _sub_path=''):
         """
         Recursively add empty directories to a tree object
 
@@ -983,7 +1156,7 @@ class GitUbuntuRepository:
 
         repo: pygit2.Repository object
         top_path: path to the extracted contents of the tree
-        top_tree: tree object
+        top_tree_object: tree object
         _sub_path (internal): relative path for where we are for recursive call
 
         Returns None if oid unchanged, or oid if it changed.
@@ -1009,7 +1182,7 @@ class GitUbuntuRepository:
                 entry_oid = cls._add_missing_tree_dirs(
                     repo=repo,
                     top_path=top_path,
-                    top_tree=top_tree,
+                    top_tree_object=top_tree_object,
                     _sub_path=os.path.join(_sub_path, entry),
                 )
                 if entry_oid:
@@ -1021,7 +1194,7 @@ class GitUbuntuRepository:
                         # recursive call's tree object, so start one.
                         tree_builder = cls._create_replacement_tree_builder(
                             repo=repo,
-                            treeish=top_tree,
+                            treeish=top_tree_object,
                             sub_path=_sub_path,
                         )
                     # If the entry previous existed, remove it.
@@ -1067,7 +1240,7 @@ class GitUbuntuRepository:
         replacement_oid = self._add_missing_tree_dirs(
             repo=self.raw_repo,
             top_path=path,
-            top_tree=tree,
+            top_tree_object=tree,
         )
         if replacement_oid:
             # Empty directories had to be added
diff --git a/gitubuntu/importer.py b/gitubuntu/importer.py
index 9f466c9..19c6f2a 100644
--- a/gitubuntu/importer.py
+++ b/gitubuntu/importer.py
@@ -37,7 +37,14 @@ import tempfile
 import time
 from gitubuntu.cache import CACHE_PATH
 from gitubuntu.dsc import GitUbuntuDsc
-from gitubuntu.git_repository import GitUbuntuRepository, orphan_tag, applied_tag, import_tag, upstream_tag
+from gitubuntu.git_repository import (
+    GitUbuntuRepository,
+    GitUbuntuRepositoryFetchError,
+    orphan_tag,
+    applied_tag,
+    import_tag,
+    upstream_tag,
+)
 from gitubuntu.run import decode_binary, run, runq
 from gitubuntu.source_information import GitUbuntuSourceInformation, NoPublicationHistoryException, SourceExtractionException, launchpad_login_auth
 from gitubuntu.version import VERSION
@@ -1165,16 +1172,22 @@ class GitUbuntuImport:
 
         self.local_repo.add_base_remotes(self.pkgname, repo_owner=owner)
         if not args.no_fetch:
-            self.local_repo.fetch_base_remotes(must_exist=False)
+            try:
+                self.local_repo.fetch_base_remotes()
+            except GitUbuntuRepositoryFetchError:
+                pass
 
         self.local_repo.delete_branches_in_namespace(self.namespace)
         self.local_repo.delete_tags_in_namespace(self.namespace)
 
         self.local_repo.copy_base_references(self.namespace)
         if not args.no_fetch:
-            self.local_repo.fetch_remote_refspecs('pkg',
-                refspecs=['refs/tags/*:refs/tags/%s/*' % self.namespace],
-                must_exist=False)
+            try:
+                self.local_repo.fetch_remote_refspecs('pkg',
+                    refspecs=['refs/tags/*:refs/tags/%s/*' % self.namespace],
+                )
+            except GitUbuntuRepositoryFetchError:
+                pass
 
         self.local_repo.ensure_importer_branches_exist(self.namespace)
 
diff --git a/gitubuntu/importppa.py b/gitubuntu/importppa.py
index e8c67da..a4a3ec1 100644
--- a/gitubuntu/importppa.py
+++ b/gitubuntu/importppa.py
@@ -6,7 +6,10 @@ import os
 import re
 import sys
 from gitubuntu.cache import CACHE_PATH
-from gitubuntu.git_repository import GitUbuntuRepository
+from gitubuntu.git_repository import (
+    GitUbuntuRepository,
+    GitUbuntuRepositoryFetchError,
+)
 from gitubuntu.importer import GitUbuntuImport
 from gitubuntu.source_information import GitUbuntuSourceInformation, NoPublicationHistoryException, SourceExtractionException, launchpad_login_auth
 from gitubuntu.version import VERSION
@@ -109,7 +112,10 @@ class GitUbuntuImportPPA(GitUbuntuImport):
 
         self.local_repo.add_remote(pkgname, owner, self.namespace, user)
         if not args.no_fetch:
-            self.local_repo.fetch_remote(self.namespace, must_exist=False)
+            try:
+                self.local_repo.fetch_remote(self.namespace)
+            except GitUbuntuRepositoryFetchError:
+                pass
 
         source_information = UbuntuSourceInformation(ppa, pkgname,
             os.path.abspath(args.pullfile),
diff --git a/gitubuntu/merge.py b/gitubuntu/merge.py
index 55df89a..6d1624f 100644
--- a/gitubuntu/merge.py
+++ b/gitubuntu/merge.py
@@ -267,15 +267,23 @@ class GitUbuntuMerge:
         # can't find the bug-specific tag
         try:
             ancestors_checked.add('%snew/debian' % self.tag_prefix)
-            self.local_repo.git_run(['merge-base', '--is-ancestor',
-                                     '%snew/debian' % self.tag_prefix,
-                                     'HEAD'], quiet=True)
+            self.local_repo.git_run(
+                [
+                    'merge-base',
+                    '--is-ancestor',
+                    '%snew/debian' % self.tag_prefix,
+                    'HEAD',
+                ],
+                verbose_on_failure=False,
+            )
             ancestor_check = True
         except subprocess.CalledProcessError as e:
             try:
                 ancestors_checked.add('new/debian')
-                self.local_repo.git_run(['merge-base', '--is-ancestor',
-                                         'new/debian', 'HEAD'], quiet=True)
+                self.local_repo.git_run(
+                    ['merge-base', '--is-ancestor', 'new/debian', 'HEAD'],
+                    verbose_on_failure=False,
+                )
                 ancestor_check = True
             except subprocess.CalledProcessError as e:
                 pass
diff --git a/gitubuntu/queue.py b/gitubuntu/queue.py
index 50674bd..d08c797 100644
--- a/gitubuntu/queue.py
+++ b/gitubuntu/queue.py
@@ -8,7 +8,10 @@ import urllib
 
 import pygit2
 
-from gitubuntu.git_repository import GitUbuntuRepository
+from gitubuntu.git_repository import (
+    GitUbuntuRepository,
+    GitUbuntuRepositoryFetchError,
+)
 import gitubuntu.importer
 import gitubuntu.source_information
 
@@ -302,7 +305,14 @@ class GitUbuntuQueue:
 
         if args.subsubcommand == 'sync':
             if args.fetch:
-                repo.fetch_base_remotes(must_exist=True)
+                try:
+                    repo.fetch_base_remotes()
+                except GitUbuntuRepositoryFetchError:
+                    logging.error('No objects found in remote %s',
+                        remote_name
+                    )
+                    sys.exit(1)
+
             self.sync(
                 repo,
                 lp,
diff --git a/gitubuntu/remote.py b/gitubuntu/remote.py
index bcd3b6d..793c421 100644
--- a/gitubuntu/remote.py
+++ b/gitubuntu/remote.py
@@ -5,7 +5,11 @@ import pygit2
 import re
 from subprocess import CalledProcessError
 import sys
-from gitubuntu.git_repository import GitUbuntuRepository, GitUbuntuChangelogError
+from gitubuntu.git_repository import (
+    GitUbuntuRepository,
+    GitUbuntuRepositoryFetchError,
+    GitUbuntuChangelogError,
+)
 from gitubuntu.run import decode_binary, run
 
 
@@ -65,7 +69,10 @@ class GitUbuntuRemote:
             )
 
         if not self.no_fetch:
-            self.local_repo.fetch_remote(self.remote_name, must_exist=False)
+            try:
+                self.local_repo.fetch_remote(self.remote_name, verbose=True)
+            except GitUbuntuRepositoryFetchError:
+                pass
 
         logging.debug("added remote '%s' -> %s", self.remote_name,
                       self.local_repo.raw_repo.remotes[self.remote_name].url)
diff --git a/gitubuntu/run.py b/gitubuntu/run.py
index 26b11db..26d9c36 100644
--- a/gitubuntu/run.py
+++ b/gitubuntu/run.py
@@ -21,16 +21,19 @@ def quoted_cmd(args):
 
 
 def runq(*args, **kwargs):
-    kwargs.update({'stdout': subprocess.DEVNULL,
-                   'stderr': subprocess.DEVNULL,
-                   'quiet': True})
+    kwargs.update({
+        'stdout': subprocess.DEVNULL,
+        'stderr': subprocess.DEVNULL,
+        'verbose_on_failure': False,
+    })
     return run(*args, **kwargs)
 
 
 def run(args, env=None, check=True, shell=False,
-        input=None,
-        stderr=subprocess.PIPE, stdout=subprocess.PIPE,
-        stdin=subprocess.DEVNULL, quiet=False, rcs=[]):
+    input=None,
+    stderr=subprocess.PIPE, stdout=subprocess.PIPE,
+    stdin=subprocess.DEVNULL, verbose_on_failure=True, rcs=[]
+):
     if shell:
         if isinstance(args, str):
             pcmd = quoted_cmd(["sh", '-c', args])
@@ -58,7 +61,7 @@ def run(args, env=None, check=True, shell=False,
             err = e.stderr.decode(errors="replace")
         if stdout is subprocess.PIPE:
             out = e.stdout.decode(errors="replace")
-        if not quiet and e.returncode not in rcs:
+        if verbose_on_failure and e.returncode not in rcs:
             logging.error("Command exited %d: %s", e.returncode, pcmd)
             logging.error("stdout: %s",
                 out.rstrip().replace("\n", "\n  "))
@@ -67,11 +70,11 @@ def run(args, env=None, check=True, shell=False,
         raise e
 
 
-def decode_binary(binary, quiet=False):
+def decode_binary(binary, verbose=True):
     try:
         return binary.decode('utf-8')
     except UnicodeDecodeError as e:
-        if not quiet:
+        if verbose:
             logging.warning("Failed to decode blob: %s", e)
             logging.warning("blob=%s", binary.decode(errors='replace'))
         return binary.decode('utf-8', errors='replace')
diff --git a/gitubuntu/versioning.py b/gitubuntu/versioning.py
index 72723d8..c3187f5 100644
--- a/gitubuntu/versioning.py
+++ b/gitubuntu/versioning.py
@@ -7,13 +7,13 @@ import sys
 from gitubuntu.source_information import GitUbuntuSourceInformation
 
 try:
-    pkg = 'python3-pytest'
-    import pytest
     pkg = 'python3-debian'
     import debian
     # This effectively declares an interface for the type of Version
     # object we want to use in the git-ubuntu code
     from debian.debian_support import NativeVersion as Version
+    pkg = 'python3-pytest'
+    import pytest
 except ImportError:
     logging.error('Is %s installed?', pkg)
     sys.exit(1)
diff --git a/tests/changelogs/test_versions_1 b/tests/changelogs/test_versions_1
new file mode 100644
index 0000000..b0e5e65
--- /dev/null
+++ b/tests/changelogs/test_versions_1
@@ -0,0 +1,5 @@
+testpkg (1.0) xenial; urgency=medium
+
+  * Dummy entry.
+
+ -- Test Maintainer <[e-mail]>  Mon, 12 May 2016 08:14:34 -0700
diff --git a/tests/changelogs/test_versions_2 b/tests/changelogs/test_versions_2
new file mode 100644
index 0000000..40a0fd6
--- /dev/null
+++ b/tests/changelogs/test_versions_2
@@ -0,0 +1,11 @@
+testpkg (2.0) xenial; urgency=medium
+
+  * Dummy entry 2.
+
+ -- Test Maintainer <[e-mail]>  Mon, 27 Aug 2016 12:10:34 -0700
+
+testpkg (1.0) xenial; urgency=medium
+
+  * Dummy entry 1.
+
+ -- Test Maintainer <[e-mail]>  Mon, 12 May 2016 08:14:34 -0700
diff --git a/tests/changelogs/test_versions_3 b/tests/changelogs/test_versions_3
new file mode 100644
index 0000000..fbbeee8
--- /dev/null
+++ b/tests/changelogs/test_versions_3
@@ -0,0 +1,23 @@
+testpkg (4.0) zesty; urgency=medium
+
+  * Dummy entry 4.
+
+ -- Test Maintainer <[e-mail]>  Mon, 03 Apr 2017 18:04:01 -0700
+
+testpkg (3.0) yakkety; urgency=medium
+
+  * Dummy entry 3.
+
+ -- Test Maintainer <[e-mail]>  Fri, 10 Nov 2016 03:34:10 -0700
+
+testpkg (2.0) xenial; urgency=medium
+
+  * Dummy entry 2.
+
+ -- Test Maintainer <[e-mail]>  Sat, 27 Aug 2016 12:10:55 -0700
+
+testpkg (1.0) xenial; urgency=medium
+
+  * Dummy entry 1.
+
+ -- Test Maintainer <[e-mail]>  Thu, 12 May 2016 08:14:34 -0700