[LIBREPORT PATCH] debuginfo code cleanup, closes #171

Petr Kubat pkubat at redhat.com
Thu Aug 29 16:11:37 UTC 2013


Signed-off-by: Petr Kubat <pkubat at redhat.com>
---
 src/client-python/debuginfo.py | 549 +++++++++++++++++++++++++++--------------
 1 file changed, 361 insertions(+), 188 deletions(-)

diff --git a/src/client-python/debuginfo.py b/src/client-python/debuginfo.py
index 07e54c2..779c25c 100644
--- a/src/client-python/debuginfo.py
+++ b/src/client-python/debuginfo.py
@@ -1,29 +1,33 @@
-from subprocess import Popen, PIPE
-from yum import _, YumBase
-from yum.callbacks import DownloadBaseCallback
-from reportclient import *
-from reportclient import _
+"""
+    This module provides classes and functions used to download and manage
+    debuginfos.
+"""
+
 import sys
 import os
 import time
 import errno
 import shutil
+from subprocess import Popen
+
+from yum import _, YumBase
+from yum.callbacks import DownloadBaseCallback
+from yum.Errors import YumBaseError
+
+from reportclient import (_, log1, log2, RETURN_OK, RETURN_FAILURE,
+                          RETURN_CANCEL_BY_USER, verbose, ask_yes_no,
+                          error_msg)
 
-old_stdout = -1
-def mute_stdout():
-    if verbose < 2:
-        global old_stdout
-        old_stdout = sys.stdout
-        sys.stdout = open("/dev/null", "w")
-
-def unmute_stdout():
-    if verbose < 2:
-        if old_stdout != -1:
-            sys.stdout = old_stdout
-        else:
-            print "ERR: unmute called without mute?"
 
-def ensure_abrt_uid(fn):
+def ensure_abrt_uid(func):
+    """
+    Ensures that the function is called using abrt's uid and gid
+
+    Returns:
+        Either an unchanged function object or a wrapper function object for
+        the function.
+    """
+
     import pwd
     current_uid = os.getuid()
     current_gid = os.getgid()
@@ -31,14 +35,24 @@ def ensure_abrt_uid(fn):
 
     # if we're are already running as abrt, don't do anything
     if abrt.pw_uid == current_uid and abrt.pw_gid == current_gid:
-        return fn
+        return func
 
     def wrapped(*args, **kwargs):
+        """
+        Wrapper function around the called function.
+
+        Sets up uid and gid to match abrt's and after the function finishes
+        rolls its uid and gid back.
+
+        Returns:
+            Return value of the wrapped function.
+        """
+
         # switch to abrt
         os.setegid(abrt.pw_gid)
         os.seteuid(abrt.pw_uid)
         # extract the files as abrt:abrt
-        retval = fn(*args, **kwargs)
+        retval = func(*args, **kwargs)
         # switch back to whatever we were
         os.seteuid(current_uid)
         os.setegid(current_gid)
@@ -46,11 +60,28 @@ def ensure_abrt_uid(fn):
 
     return wrapped
 
+
 # TODO: unpack just required debuginfo and not entire rpm?
 # ..that can lead to: foo.c No such file and directory
 # files is not used...
 @ensure_abrt_uid
-def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm, exact_files=False):
+def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm,
+               exact_files=False):
+    """
+    Unpacks a single rpm located in tmp_dir into destdir.
+
+    Arguments:
+        package_file_name - name of the rpm file
+        files - files to extract from the rpm
+        tmp_dir - temporary directory where the rpm file is located
+        destdir - destination directory for the rpm package extraction
+        keeprpm - check if the user wants to delete rpms from the tmp directory
+        exact_files - extract only specified files
+
+    Returns:
+        RETURN_FAILURE in case of a serious problem
+    """
+
     package_full_path = tmp_dir + "/" + package_file_name
     log1("Extracting %s to %s", package_full_path, destdir)
     log2("%s", files)
@@ -63,7 +94,7 @@ def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm, exact_files=
         return RETURN_FAILURE
 
     rpm2cpio = Popen(["rpm2cpio", package_full_path],
-                       stdout = unpacked_cpio, bufsize = -1)
+                     stdout=unpacked_cpio, bufsize=-1)
     retcode = rpm2cpio.wait()
 
     if retcode == 0:
@@ -82,7 +113,8 @@ def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm, exact_files=
     # and open it for reading
     unpacked_cpio = open(unpacked_cpio_path, 'rb')
 
-    print _("Caching files from {0} made from {1}").format("unpacked.cpio", package_file_name)
+    print _("Caching files from {0} made from {1}").format("unpacked.cpio",
+                                                           package_file_name)
 
     file_patterns = ""
     cpio_args = ["cpio", "-idu"]
@@ -104,16 +136,20 @@ def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm, exact_files=
         print _("Can't extract files from '{0}'").format(unpacked_cpio_path)
         return RETURN_FAILURE
 
-def clean_up():
-    if tmpdir:
-        try:
-            shutil.rmtree(tmpdir)
-        except OSError, ex:
-            if ex.errno != errno.ENOENT:
-                error_msg(_("Can't remove '{0}': {1}").format(tmpdir, ex))
 
 class MyDownloadCallback(DownloadBaseCallback):
+    """
+    This class serves as a download progress handler for yum's progress bar.
+    """
+
     def __init__(self, total_pkgs):
+        """
+        Sets up instance variables
+
+        Arguments:
+            total_pkgs - number of packages to download
+        """
+
         self.total_pkgs = total_pkgs
         self.downloaded_pkgs = 0
         self.last_pct = 0
@@ -121,6 +157,16 @@ class MyDownloadCallback(DownloadBaseCallback):
         DownloadBaseCallback.__init__(self)
 
     def updateProgress(self, name, frac, fread, ftime):
+        """
+        A method used to update the progress
+
+        Arguments:
+            name - filename
+            frac - progress fracment (0 -> 1)
+            fread - formated string containing BytesRead
+            ftime - formated string containing remaining or elapsed time
+        """
+
         pct = int(frac * 100)
         if pct == self.last_pct:
             log2("percentage is the same, not updating progress")
@@ -130,50 +176,57 @@ class MyDownloadCallback(DownloadBaseCallback):
         # if run from terminal we can have fancy output
         if sys.stdout.isatty():
             sys.stdout.write("\033[sDownloading (%i of %i) %s: %3u%%\033[u"
-                    % (self.downloaded_pkgs + 1, self.total_pkgs, name, pct)
-            )
+                             % (self.downloaded_pkgs + 1, self.total_pkgs,
+                                name, pct))
             if pct == 100:
                 #print (_("Downloading (%i of %i) %s: %3u%%")
-                #        % (self.downloaded_pkgs + 1, self.total_pkgs, name, pct)
+                #      % (self.downloaded_pkgs + 1, self.total_pkgs, name, pct)
                 #)
                 print (_("Downloading ({0} of {1}) {2}: {3:3}%").format(
-                        self.downloaded_pkgs + 1, self.total_pkgs, name, pct
-                        )
-                )
+                       self.downloaded_pkgs + 1, self.total_pkgs, name, pct))
         # but we want machine friendly output when spawned from abrt-server
         else:
-            t = time.time()
+            cur_time = time.time()
             if self.last_time == 0:
-                self.last_time = t
+                self.last_time = cur_time
             # update only every 5 seconds
-            if pct == 100 or self.last_time > t or t - self.last_time >= 5:
+            if pct == 100 or (self.last_time > cur_time or
+                              cur_time - self.last_time >= 5):
                 print (_("Downloading ({0} of {1}) {2}: {3:3}%").format(
-                        self.downloaded_pkgs + 1, self.total_pkgs, name, pct
-                        )
-                )
-                self.last_time = t
+                       self.downloaded_pkgs + 1, self.total_pkgs, name, pct))
+                self.last_time = cur_time
                 if pct == 100:
                     self.last_time = 0
 
         sys.stdout.flush()
 
-def downloadErrorCallback(callBackObj):
-    print _("Problem '{0!s}' occured while downloading from mirror: '{1!s}'. Trying next one").format(
-        callBackObj.exception, callBackObj.mirror)
+
+def download_error_callback(callback_obj):
+    """
+    A callback function for mirror errors.
+    """
+
+    print _("Problem '{0!s}' occured while downloading from mirror: '{1!s}'."
+            "Trying next one").format(
+        callback_obj.exception, callback_obj.mirror)
     # explanation of the return value can be found here:
     # /usr/lib/python2.7/site-packages/urlgrabber/mirror.py
-    return {'fail':0}
+    return {'fail': 0}
+
 
 class DebugInfoDownload(YumBase):
+    """
+    This class is used to manage download of debuginfos.
+    """
+
     def __init__(self, cache, tmp, keep_rpms=False, noninteractive=True):
+        self.old_stdout = -1
         self.cachedir = cache
         self.tmpdir = tmp
-        global tmpdir
-        tmpdir = tmp
         self.keeprpms = keep_rpms
         self.noninteractive = noninteractive
         YumBase.__init__(self)
-        mute_stdout()
+        self.mute_stdout()
         #self.conf.cache = os.geteuid() != 0
         # Setup yum (Ts, RPM db, Repo & Sack)
         # doConfigSetup() takes some time, let user know what we are doing
@@ -181,17 +234,59 @@ class DebugInfoDownload(YumBase):
         try:
             # Saw this exception here:
             # cannot open Packages index using db3 - Permission denied (13)
-            # yum.Errors.YumBaseError: Error: rpmdb open failed
+            # YumBaseError: Error: rpmdb open failed
             self.doConfigSetup()
-        except Exception, e:
-            unmute_stdout()
-            print _("Error initializing yum (YumBase.doConfigSetup): '{0!s}'").format(e)
+        except YumBaseError, ex:
+            self.unmute_stdout()
+            print _("Error initializing yum (YumBase.doConfigSetup): "
+                    "'{0!s}'").format(ex)
             #return 1 - can't do this in constructor
             exit(1)
-        unmute_stdout()
+        self.unmute_stdout()
+
+    def mute_stdout(self):
+        """
+        Links sys.stdout with /dev/null and saves the old stdout
+        """
+
+        if verbose < 2:
+            self.old_stdout = sys.stdout
+            sys.stdout = open("/dev/null", "w")
+
+    def unmute_stdout(self):
+        """
+        Replaces sys.stdout by stdout saved using mute
+        """
+
+        if verbose < 2:
+            if self.old_stdout != -1:
+                sys.stdout = self.old_stdout
+            else:
+                print "ERR: unmute called without mute?"
+
+    def cleanup_tmp_dir(self):
+        """
+        Removes the temporary directory.
+        """
+
+        if self.tmpdir:
+            try:
+                shutil.rmtree(self.tmpdir)
+            except OSError, ex:
+                if ex.errno != errno.ENOENT:
+                    error_msg(_("Can't remove '{0}': {1}").format(
+                        self.tmpdir, ex))
 
     @ensure_abrt_uid
     def setup_tmp_dirs(self):
+        """
+        Sets up tmpdir and cache dir if they are not set up already.
+
+        Returns:
+            RETURN_OK if all goes well.
+            RETURN_FAILURE in case it cannot set up either of the directories.
+        """
+
         if not os.path.exists(self.tmpdir):
             try:
                 os.makedirs(self.tmpdir)
@@ -207,83 +302,219 @@ class DebugInfoDownload(YumBase):
 
         return RETURN_OK
 
-    # return value will be used as exitcode. So 0 = ok, !0 - error
-    def download(self, files, download_exact_files=False):
-        """ @files - """
-        installed_size = 0
-        total_pkgs = 0
-        todownload_size = 0
+    def _download_rpms(self, package_files_dict, total_pkgs,
+                       download_exact_files):
+        """
+        Downloads rpms into a temporary directory
+
+        Arguments:
+            package_files_dict - a dict containing {pkg: file list} entries
+            total_pkgs - total number of packages to download
+            download_exact_files - extract only specified files
+
+        Returns:
+            RETURN_OK if all goes well.
+            RETURN_FAILURE in case it cannot set up either of the directories.
+        """
+
+        # connect our progress update callback
+        dnlcb = MyDownloadCallback(total_pkgs)
+        self.repos.setProgressBar(dnlcb)
+        self.repos.setMirrorFailureCallback(download_error_callback)
         downloaded_pkgs = 0
-        # nothing to download?
-        if not files:
-            return RETURN_FAILURE
 
-        #if verbose == 0:
-        #    # this suppress yum messages about setting up repositories
-        #    mute_stdout()
+        for pkg, files in package_files_dict.iteritems():
+            dnlcb.downloaded_pkgs = downloaded_pkgs
+            remote = pkg.returnSimple('relativepath')
+            local = os.path.basename(remote)
+            retval = self.setup_tmp_dirs()
+            # continue only if the tmp dirs are ok
+            if retval != RETURN_OK:
+                return retval
 
-        # make yumdownloader work as non root user
-        if not self.setCacheDir():
-            print _("Error: can't make cachedir, exiting")
-            return RETURN_FAILURE
+            remote = pkg.returnSimple('remote_url')
+            # check if the pkg is in a local repo and copy it if it is
+            err = None
+            if remote.startswith('file:///'):
+                log2("copying from local repo: %s", remote)
+                try:
+                    shutil.copy(remote[7:], self.tmpdir)
+                except OSError, ex:
+                    print _("Cannot copy file '{0}': {1}").format(
+                        remote[7:], ex)
+                    continue
+            else:
+                # pkg is in a remote repo, we need to download it to tmpdir
+                local = os.path.join(self.tmpdir, local)
+                pkg.localpath = local  # Hack: to set the localpath we want
+                err = self.downloadPkgs(pkglist=[pkg])
+            # normalize the name
+            # just str(pkg) doesn't work because it can have epoch
+            package_file_name = (pkg.name + "-" + pkg.version + "-" +
+                                 pkg.release + "." + pkg.arch) + ".rpm"
+            if err:
+                # I observed a zero-length file left on error,
+                # which prevents cleanup later. Fix it:
+                try:
+                    os.unlink(self.tmpdir + "/" + package_file_name)
+                except OSError:
+                    pass
+                print (_("Downloading package {0} failed").format(pkg))
+            else:
+                unpack_result = unpack_rpm(package_file_name, files,
+                                           self.tmpdir, self.cachedir,
+                                           self.keeprpms,
+                                           exact_files=download_exact_files)
+                if unpack_result == RETURN_FAILURE:
+                    # recursively delete the temp dir on failure
+                    print _("Unpacking failed, aborting download...")
+                    self.cleanup_tmp_dir()
+                    return RETURN_FAILURE
+
+            downloaded_pkgs += 1
+        return RETURN_OK
+
+    def _setup_repos(self):
+        """
+        Disables repos and then sets up repos one-by-one so we can skip
+        broken ones.
+
+        this helps when users are using 3rd party repos like rpmfusion
+        in rawhide it results in: Can't find valid base url
+        """
 
         # disable all not needed
         for repo in self.repos.listEnabled():
             try:
                 repo.close()
                 self.repos.disableRepo(repo.id)
-            except Exception, ex:
-                print _("Can't disable repository '{0!s}': {1!s}").format(repo.id, str(ex))
+            except YumBaseError, ex:
+                print _("Can't disable repository '{0!s}': {1!s}").format(
+                    repo.id, str(ex))
 
         # This takes some time, let user know what we are doing
         print _("Setting up yum repositories")
-        # setting-up repos one-by-one, so we can skip the broken ones...
-        # this helps when users are using 3rd party repos like rpmfusion
-        # in rawhide it results in: Can't find valid base url...
-        for r in self.repos.findRepos(pattern="*debug*"):
+        for repo in self.repos.findRepos(pattern="*debug*"):
             try:
-                rid = self.repos.enableRepo(r.id)
-                self.repos.doSetup(thisrepo=str(r.id))
+                rid = self.repos.enableRepo(repo.id)
+                self.repos.doSetup(thisrepo=str(repo.id))
                 log1("enabled repo %s", rid)
-                setattr(r, "skip_if_unavailable", True)
+                setattr(repo, "skip_if_unavailable", True)
                 # yes, we want async download, otherwise our progressCallback
                 # is not called and the internal yum's one  is used,
                 # which causes artifacts on output
                 try:
-                    setattr(r, "_async", False)
-                except Exception, ex:
+                    setattr(repo, "_async", False)
+                except (NameError, AttributeError), ex:
                     print ex
-                    print _("Can't disable async download, the output might contain artifacts!")
-            except Exception, ex:
-                print _("Can't setup {0}: {1}, disabling").format(r.id, ex)
-                self.repos.disableRepo(r.id)
+                    print _("Can't disable async download, the output might"
+                            " contain artifacts!")
+            except (NameError, AttributeError, YumBaseError), ex:
+                print _("Can't setup {0}: {1}, disabling").format(repo.id, ex)
+                self.repos.disableRepo(repo.id)
+
+    def _check_space(self, total_pkgs, todownload_size, installed_size):
+        """
+        Checks free space in tmp and cache directories.
+
+        Returns:
+            RETURN_OK if all goes well.
+            RETURN_FAILURE in case it cannot set up either of the directories.
+        """
+
+        if verbose != 0 or total_pkgs != 0:
+            print _("Packages to download: {0}").format(total_pkgs)
+            question = _("Downloading {0:.2f}Mb, installed size: {1:.2f}Mb."
+                         " Continue?").format(
+                todownload_size / (1024*1024), installed_size / (1024*1024))
+            if not self.noninteractive and not ask_yes_no(question):
+                print _("Download cancelled by user")
+                return RETURN_CANCEL_BY_USER
+
+            # set up tmp and cache dirs so that we can check free space in both
+            retval = self.setup_tmp_dirs()
+            if retval != RETURN_OK:
+                return retval
+            # check if there is enough free space in both tmp and cache
+            res = os.statvfs(self.tmpdir)
+            tmp_space = float(res.f_bsize * res.f_bavail) / (1024*1024)
+            if (todownload_size / (1024*1024)) > tmp_space:
+                question = _("Warning: Not enough free space in tmp dir '{0}'"
+                             " ({1:.2f}Mb left). Continue?").format(
+                    self.tmpdir, tmp_space)
+                if not self.noninteractive and not ask_yes_no(question):
+                    print _("Download cancelled by user")
+                    return RETURN_CANCEL_BY_USER
+            res = os.statvfs(self.cachedir)
+            cache_space = float(res.f_bsize * res.f_bavail) / (1024*1024)
+            if (installed_size / (1024*1024)) > cache_space:
+                question = _("Warning: Not enough free space in cache dir "
+                             "'{0}' ({1:.2f}Mb left). Continue?").format(
+                    self.cachedir, cache_space)
+                if not self.noninteractive and not ask_yes_no(question):
+                    print _("Download cancelled by user")
+                    return RETURN_CANCEL_BY_USER
+
+        return RETURN_OK
+
+    def _retrieve_repo_data(self):
+        """
+        Retrieves metadata and filelists from remote servers.
+        """
 
-        # This is somewhat "magic", it unpacks the metadata making it usable.
-        # Looks like this is the moment when yum talks to remote servers,
-        # which takes time (sometimes minutes), let user know why
-        # we have "paused":
-        print _("Looking for needed packages in repositories")
         try:
             self.repos.populateSack(mdtype='metadata', cacheonly=1)
-        except Exception, e:
-            print _("Error retrieving metadata: '{0!s}'").format(e)
-            #we don't want to die here, some metadata might be already retrieved
-            # so there is a chance we already have what we need
-            #return 1
+        except YumBaseError, ex:
+            print _("Error retrieving metadata: '{0!s}'").format(ex)
+            # we don't want to die here, some metadata might be already
+            # retrieved so there is a chance we already have what we need
+            # return 1
 
         try:
             # Saw this exception here:
             # raise Errors.NoMoreMirrorsRepoError, errstr
             # NoMoreMirrorsRepoError: failure:
-            # repodata/7e6632b82c91a2e88a66ad848e231f14c48259cbf3a1c3e992a77b1fc0e9d2f6-filelists.sqlite.bz2
             # from fedora-debuginfo: [Errno 256] No more mirrors to try.
             self.repos.populateSack(mdtype='filelists', cacheonly=1)
-        except Exception, ex:
+        except YumBaseError, ex:
             print _("Error retrieving filelists: '{0!s}'").format(ex)
             # we don't want to die here, some repos might be already processed
             # so there is a chance we already have what we need
             #return 1
 
+    # return value will be used as exitcode. So 0 = ok, !0 - error
+    def download(self, files, download_exact_files=False):
+        """
+        Finds debuginfo files in an rpm package and downloads it to
+        a temporary directory.
+        """
+
+        installed_size = 0
+        total_pkgs = 0
+        todownload_size = 0
+        # nothing to download?
+        if not files:
+            return RETURN_FAILURE
+
+        #if verbose == 0:
+        #    # this suppress yum messages about setting up repositories
+        #    mute_stdout()
+
+        # make yumdownloader work as non root user
+        if not self.setCacheDir():
+            print _("Error: can't make cachedir, exiting")
+            return RETURN_FAILURE
+
+        # set up yum repositories
+        self._setup_repos()
+
+        # This is somewhat "magic", it unpacks the metadata making it usable.
+        # Looks like this is the moment when yum talks to remote servers,
+        # which takes time (sometimes minutes), let user know why
+        # we have "paused":
+        print _("Looking for needed packages in repositories")
+        self._retrieve_repo_data()
+
         #if verbose == 0:
         #    # re-enable the output to stdout
         #    unmute_stdout()
@@ -309,94 +540,18 @@ class DebugInfoDownload(YumBase):
                 log2("not found pkg for %s", debuginfo_path)
                 not_found.append(debuginfo_path)
 
-        # connect our progress update callback
-        dnlcb = MyDownloadCallback(total_pkgs)
-        self.repos.setProgressBar(dnlcb)
-        self.repos.setMirrorFailureCallback(downloadErrorCallback)
-
         if verbose != 0 or len(not_found) != 0:
-            print _("Can't find packages for {0} debuginfo files").format(len(not_found))
-        if verbose != 0 or total_pkgs != 0:
-            print _("Packages to download: {0}").format(total_pkgs)
-            question = _("Downloading {0:.2f}Mb, installed size: {1:.2f}Mb. Continue?").format(
-                         todownload_size / (1024*1024),
-                         installed_size / (1024*1024)
-                        )
-            if self.noninteractive == False and not ask_yes_no(question):
-                print _("Download cancelled by user")
-                return RETURN_CANCEL_BY_USER
-            # set up tmp and cache dirs so that we can check free space in both
-            retval = self.setup_tmp_dirs()
-            if retval != RETURN_OK:
-                return retval
-            # check if there is enough free space in both tmp and cache
-            res = os.statvfs(self.tmpdir)
-            tmp_space = float(res.f_bsize * res.f_bavail) / (1024*1024)
-            if (todownload_size / (1024*1024)) > tmp_space:
-                question = _("Warning: Not enough free space in tmp dir '{0}'"
-                             " ({1:.2f}Mb left). Continue?").format(
-                    self.tmpdir, tmp_space)
-                if not self.noninteractive and not ask_yes_no(question):
-                    print _("Download cancelled by user")
-                    return RETURN_CANCEL_BY_USER
-            res = os.statvfs(self.cachedir)
-            cache_space = float(res.f_bsize * res.f_bavail) / (1024*1024)
-            if (installed_size / (1024*1024)) > cache_space:
-                question = _("Warning: Not enough free space in cache dir "
-                             "'{0}' ({1:.2f}Mb left). Continue?").format(
-                    self.cachedir, cache_space)
-                if not self.noninteractive and not ask_yes_no(question):
-                    print _("Download cancelled by user")
-                    return RETURN_CANCEL_BY_USER
+            print _("Can't find packages for {0} debuginfo files").format(
+                len(not_found))
 
-        for pkg, files in package_files_dict.iteritems():
-            dnlcb.downloaded_pkgs = downloaded_pkgs
-            repo.cache = 0
-            remote = pkg.returnSimple('relativepath')
-            local = os.path.basename(remote)
-            retval = self.setup_tmp_dirs()
-            # continue only if the tmp dirs are ok
-            if retval != RETURN_OK:
-                return retval
+        retval = self._check_space(total_pkgs, todownload_size, installed_size)
+        if retval != RETURN_OK:
+            return retval
 
-            remote_path = pkg.returnSimple('remote_url')
-            # check if the pkg is in a local repo and copy it if it is
-            err = None
-            if remote_path.startswith('file:///'):
-                pkg_path = remote_path[7:]
-                log2("copying from local repo: %s", remote)
-                try:
-                    shutil.copy(pkg_path, self.tmpdir)
-                except OSError, ex:
-                    print _("Cannot copy file '{0}': {1}").format(pkg_path, ex)
-                    continue
-            else:
-                # pkg is in a remote repo, we need to download it to tmpdir
-                local = os.path.join(self.tmpdir, local)
-                pkg.localpath = local # Hack: to set the localpath we want
-                err = self.downloadPkgs(pkglist=[pkg])
-            # normalize the name
-            # just str(pkg) doesn't work because it can have epoch
-            pkg_nvra = pkg.name + "-" + pkg.version + "-" + pkg.release + "." + pkg.arch
-            package_file_name = pkg_nvra + ".rpm"
-            if err:
-                # I observed a zero-length file left on error,
-                # which prevents cleanup later. Fix it:
-                try:
-                    os.unlink(self.tmpdir + "/" + package_file_name)
-                except:
-                    pass
-                print (_("Downloading package {0} failed").format(pkg))
-            else:
-                unpack_result = unpack_rpm(package_file_name, files, self.tmpdir,
-                                           self.cachedir, self.keeprpms, exact_files=download_exact_files)
-                if unpack_result == RETURN_FAILURE:
-                    # recursively delete the temp dir on failure
-                    print _("Unpacking failed, aborting download...")
-                    clean_up()
-                    return RETURN_FAILURE
-
-            downloaded_pkgs += 1
+        retval = self._download_rpms(package_files_dict, total_pkgs,
+                                     download_exact_files)
+        if retval != RETURN_OK:
+            return retval
 
         if not self.keeprpms and os.path.exists(self.tmpdir):
             # Was: "All downloaded packages have been extracted, removing..."
@@ -406,21 +561,39 @@ class DebugInfoDownload(YumBase):
             try:
                 os.rmdir(self.tmpdir)
             except OSError:
-                error_msg(_("Can't remove %s, probably contains an error log").format(self.tmpdir))
+                error_msg(_("Can't remove %s, probably contains an "
+                            "error log").format(self.tmpdir))
 
         return RETURN_OK
 
+
 def build_ids_to_path(pfx, build_ids):
     """
+    Transforms build ids into a path.
+
     build_id1=${build_id:0:2}
     build_id2=${build_id:2}
     file="usr/lib/debug/.build-id/$build_id1/$build_id2.debug"
     """
-    return ["%s/usr/lib/debug/.build-id/%s/%s.debug" % (pfx, b_id[:2], b_id[2:]) for b_id in build_ids]
+
+    return ["%s/usr/lib/debug/.build-id/%s/%s.debug" %
+            (pfx, b_id[:2], b_id[2:]) for b_id in build_ids]
 
 # beware this finds only missing libraries, but not the executable itself ..
 
+
 def filter_installed_debuginfos(build_ids, cache_dirs):
+    """
+    Checks for installed debuginfos.
+
+    Arguments:
+        build_ids - string containing build ids
+        cache_dirs - list of cache directories
+
+    Returns:
+        List of missing debuginfo files.
+    """
+
     files = build_ids_to_path("", build_ids)
     missing = []
 
@@ -436,7 +609,7 @@ def filter_installed_debuginfos(build_ids, cache_dirs):
     if missing:
         files = missing
         missing = []
-    else: # nothing is missing, we can stop looking
+    else:  # nothing is missing, we can stop looking
         return missing
 
     for cache_dir in cache_dirs:
@@ -454,7 +627,7 @@ def filter_installed_debuginfos(build_ids, cache_dirs):
         if missing:
             files = missing
             missing = []
-        else: # nothing is missing, we can stop looking
+        else:  # nothing is missing, we can stop looking
             return missing
 
     return files
-- 
1.8.3.1



More information about the Crash-catcher mailing list