Fixups for code-review

bzr-revno: 18.2.20

1 files changed, 46 insertions, 57 deletions

diff --git a/mfdiff b/mfdiff
index 216711a..a6787cc 100755
--- a/mfdiff
+++ b/mfdiff
@@ -76,19 +76,19 @@ def get_bin2src(packages, cache):
 return ret
 
 
-def getchangelog(source, version, cache_d):
+def get_changelog(source, version, changelog_cache_d):
 """
 Download changelog for source / version and returns path to that
 
 :param str source: Source package name
 :param str version: Source package version
- :param str cache_d: apt cache path
+ :param str changelog_cache_d: path to store cached changelogs
 :raises MissingChangelogError: If changelog file could not be downloaded
 :return: changelog file for source package & version
 :rtype: str
 """
 
- cache_f = "%s/changelog.%s_%s" % (cache_d, source, version)
+ cache_f = "%s/changelog.%s_%s" % (changelog_cache_d, source, version)
 
 if os.path.isfile(cache_f):
 logging.debug("Using cached changelog for %s:%s", source, version)
@@ -97,7 +97,7 @@ def getchangelog(source, version, cache_d):
 furls = []
 num_colon_m = re.compile("[0-9]:")
 
- cache_tmp = "%s/.changelog.%s_%s" % (cache_d, source, version)
+ cache_tmp = "%s/.changelog.%s_%s" % (changelog_cache_d, source, version)
 for pile in ("main", "universe", "multiverse", "restricted"):
 pre = source[0:1]
 # packages starting with 'lib' are special
@@ -106,24 +106,23 @@ def getchangelog(source, version, cache_d):
 
 # packages with '1:' versions have different paths
 # update-manager at version 1:0.134.11.
- # would be at main/u/update-manager/update-manager_1:0.134.11
- # instead at main/u/update-manager/update-manager_0.134.11/
+ # is main/u/update-manager/update-manager_0.134.11/
+ # rather than main/u/update-manager/update-manager_1:0.134.11
 url_version = version
 if num_colon_m.match(version):
 url_version = version[2:]
 
 # Changelog URL example http://changelogs.ubuntu.com/changelogs/\
 # pool/main/h/hal/hal_0.5.5.1-1ubuntu2/changelog
- clog_url = "%s/%s/%s/%s/%s_%s/changelog" % \
- ("http://changelogs.ubuntu.com/changelogs/pool",
- pile, pre, source, source, url_version)
+ clog_url = "http://changelogs.ubuntu.com/changelogs/pool/" \
+ "%s/%s/%s/%s_%s/changelog" % \
+ (pile, pre, source, source, url_version)
 
 changelog = requests.get(clog_url)
 if changelog.status_code == 200:
 logging.debug("Found changelog for %s in %s", source, pile)
 with open(cache_tmp, "w") as cache_file:
- cache_file.write(
- changelog.text.encode('ascii', 'replace').decode('ascii'))
+ cache_file.write(changelog.content)
 os.rename(cache_tmp, cache_f)
 return cache_f
 else:
@@ -136,7 +135,7 @@ def getchangelog(source, version, cache_d):
 ' '.join(furls)))
 
 
-def hashmffile(filename):
+def manifest_to_dict(filename):
 """
 Parse manifest file to create a package / version mapping
 
@@ -154,21 +153,19 @@ def hashmffile(filename):
 return ret
 
 
-def filecontents(filename):
- """Read file contents into a string and return the string"""
- with open(filename, "r") as fileptr:
- return fileptr.read()
-
-
-def prep_cacheroot(arch, release, cache_d=None):
+def open_apt_cache(arch, release, cache_d=None):
 """
- Create the apt cache directory and write a sources.list file
+ Create, update, and open an apt cache.
+
+ This creates an apt cache directory and write a sources.list file
+ before updating and opening the cache. The caller is responsible
+ for closing the cache.
 
 :param str arch: Package architecture
 :param str release: Ubuntu release name (e.g. Xenial)
 :param str cache_d: apt cache path
- :returns: path name for apt cache, defaults to ./cache-{release}-{arch}/
- :rtype: str
+ :returns: tuple of Open/updated apt cache and cache path name
+ :rtype: tuple(:class:`apt.Cache`, str)
 """
 
 if not cache_d:
@@ -201,24 +198,12 @@ def prep_cacheroot(arch, release, cache_d=None):
 apt.apt_pkg.config.set("Dir::Etc::TrustedParts",
 "/etc/apt/trusted.gpg.d/")
 
- return cache_d
-
-
-def get_cache(cache_d):
- """
- Create, update, and open an apt cache
-
- :param str cache_d: apt cache path
- :returns: Open/updated apt cache
- :rtype: :class:`apt.Cache`
- """
-
 cache = apt.Cache(rootdir=cache_d)
 logging.info('Updating apt cache')
 cache.update()
 logging.info('Update of apt cache complete')
 cache.open()
- return cache
+ return cache, cache_d
 
 
 def render_block(block):
@@ -272,7 +257,7 @@ def kernel_fixups(manifest_from, manifest_to):
 # linux-image-2.6.32-32-virtual)
 if kmatch.match(pkg):
 logging.debug('Found kernel %s in manifest #2', pkg)
- img_type = pkg[pkg.rfind("-") + 1:]
+ img_type = pkg.split("-")[-1]
 if pkg in manifest_from:
 logging.debug('Found same kernel in manifest #1')
 continue
@@ -352,39 +337,41 @@ def source_version_for_binary(cache, binary, binary_ver):
 raise UnknownSourceVersionError(msg)
 
 
-def filter_changelog(changelog_path, version_start, version_end):
+def filter_changelog(changelog_path, version_low, version_high):
 """
 Extract changelog entries within a version range
 
 The range of changelog entries returned will include all entries
- after version_start up to, and including, version_end.
+ after version_low up to, and including, version_high.
 If either the starting or ending version are not found in the
 list of changelog entries the result will be incomplete and
 a non-empty error message is returned to indicate the issue.
 
-
 :param str changelog_path: File name of the changelog to process
 :return: list of changelog blocks and an error_msg if incomplete
- :rtype list:
+ :rtype tuple(list, str):
 """
 
- chlog = Changelog(filecontents(changelog_path))
+ with open(changelog_path, "r") as fileptr:
+ chlog = Changelog(fileptr.read())
 change_blocks = []
 start = False
 end = False
 error_msg = ''
+
+ # The changelog blocks are in reverse order; we'll see high before low.
 for block in chlog:
- if block.version == version_end:
+ if block.version == version_high:
 start = True
 change_blocks = []
- if block.version == version_start:
+ if block.version == version_low:
 end = True
 break
 change_blocks.append(block)
 if not start:
 error_msg = "Missing starting version {} in {}. " \
 "Changlelog will be incomplete".format(
- version_start, changelog_path)
+ version_high, changelog_path)
 logging.error(error_msg)
 if not end:
 if error_msg:
@@ -393,17 +380,19 @@ def filter_changelog(changelog_path, version_start, version_end):
 error_msg += '\n'
 error_msg += "Missing ending version {} in {}. " \
 "Changelog output truncated".format(
- version_end, changelog_path)
+ version_low, changelog_path)
 logging.error(error_msg)
 return change_blocks, error_msg
 
 
-def print_changelogs(cache_d, manifest_from, manifest_to, changed):
+def print_changelogs(apt_cache, apt_cache_d, manifest_from, manifest_to,
+ changed):
 """
 Print changelog entries for each changed package limited to
 changes in the package between the versions in the two manifests.
 
- :param str cache_d: Path to the apt cache directory
+ :param :class:`apt.Cache` apt_cache: Open & up-to-date apt cache
+ :param str apt_cache_d: apt cache path
 :param dict manifest_from: Packages and their versions in the
 first manifest file
 :param dict manifest_to: Packages and their versions in the
@@ -413,8 +402,7 @@ def print_changelogs(cache_d, manifest_from, manifest_to, changed):
 
 srcs = {}
 errors = []
- cache = get_cache(cache_d)
- src2bins = map_source_to_binary(cache, changed)
+ src2bins = map_source_to_binary(apt_cache, changed)
 
 # Generate changelog data per unique source package
 for source_name in src2bins:
@@ -427,7 +415,7 @@ def print_changelogs(cache_d, manifest_from, manifest_to, changed):
 # Find the source version data for the binary in manifest #2
 try:
 src['version_to'] = source_version_for_binary(
- cache, binary_name, manifest_to[binary_name])
+ apt_cache, binary_name, manifest_to[binary_name])
 except UnknownSourceVersionError as excp:
 logging.error(excp.message)
 errors.append(excp)
@@ -437,7 +425,7 @@ def print_changelogs(cache_d, manifest_from, manifest_to, changed):
 binver_from = manifest_from[binary_name]
 try:
 src['version_from'] = source_version_for_binary(
- cache, binary_name, binver_from)
+ apt_cache, binary_name, binver_from)
 except UnknownSourceVersionError as excp:
 if manifest_to[binary_name] == src['version_to']:
 logging.info('Could not find source version data in apt '
@@ -462,8 +450,9 @@ def print_changelogs(cache_d, manifest_from, manifest_to, changed):
 
 # Get the changelog for this source package
 try:
- srcs[source_name]["changelog_file"] = getchangelog(
- source_name, src['version_to'], cache_d)
+ # Use the apt cache directory to store the changelog cache
+ srcs[source_name]["changelog_file"] = get_changelog(
+ source_name, src['version_to'], changelog_cache_d=apt_cache_d)
 except MissingChangelogError as excp:
 errors.append(excp)
 continue
@@ -552,8 +541,8 @@ def main():
 setup_logging(options.loglevel)
 
 # index both manifests
- manifest_to = hashmffile(manifest_to_filename)
- manifest_from = kernel_fixups(hashmffile(manifest_from_filename),
+ manifest_to = manifest_to_dict(manifest_to_filename)
+ manifest_from = kernel_fixups(manifest_to_dict(manifest_from_filename),
 manifest_to)
 
 new = find_added(manifest_from, manifest_to)
@@ -566,8 +555,8 @@ def main():
 
 # if modified packages, download all changelogs from changelogs.
 if changed:
- cache_d = prep_cacheroot(arch, release, options.cache_d)
- print_changelogs(cache_d, manifest_from, manifest_to, changed)
+ cache, cache_d = open_apt_cache(arch, release, options.cache_d)
+ print_changelogs(cache, cache_d, manifest_from, manifest_to, changed)
 
 
 if __name__ == "__main__":