[LIBREPORT PATCH] debuginfo code cleanup, closes #171

Jiri Moskovcak jmoskovc at redhat.com
Fri Aug 30 06:10:26 UTC 2013


I'm sorry, but I have to NACK this patch, it changes the code too much 
becuase of the code style and that's not good. We can slowly fix the 
style while fixing other bugs in the code, but changes in the working 
code to just fix the coding are not ok.

What you can do is:
- move the some functions from global space to it's only user (separate 
commit please)
- add comments

Can't:
- rename variables/functions etc
- wrap/re indent lines
- move the code around just to make pylint happy

These problems should have been catch by the review, but since it's made 
it's way to the repo we have to live with it and hope there will be some 
some necessary changes which will allow justify the changes to make it 
more pylint compatible.

--Jirka


On 08/29/2013 06:11 PM, Petr Kubat wrote:
> 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
>



More information about the Crash-catcher mailing list