apport-retrace's build sandbox routine carries on if it can't find the package for an ExecutablePath

Bug #1487174 reported by Brian Murray
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Apport
Fix Released
Undecided
Unassigned
apport (Ubuntu)
Fix Released
High
Martin Pitt

Bug Description

In the event that a package which provides an ExecutablePath or InterpreterPath can not be found apport will carry on building the sandbox but then exits a short while later (the last line in the pasted code). I think it'd be better if apport just quit earlier.

Here's the code in question from sandboxutils.py:

    # package hooks might reassign Package:, check that we have the originally
    # crashing binary
    for path in ('InterpreterPath', 'ExecutablePath'):
        if path in report:
            pkg = apport.packaging.get_file_package(report[path], True, pkgmap_cache_dir,
                                                    release=report['DistroRelease'],
                                                    arch=report.get('Architecture'))
            if pkg:
                apport.log('Installing extra package %s to get %s' % (pkg, path), log_timestamps)
                pkgs.append((pkg, pkg_versions.get(pkg)))
            else:
                apport.warning('Cannot find package which ships %s', path)

    # unpack packages for executable using cache and sandbox
    if pkgs:
        try:
            outdated_msg += apport.packaging.install_packages(
                sandbox_dir, config_dir, report['DistroRelease'], pkgs,
                verbose, cache_dir, permanent_rootdir,
                architecture=report.get('Architecture'), origins=origins)
        except SystemError as e:
            apport.fatal(str(e))

    # sanity check: for a packaged binary we require having the executable in
    # the sandbox; TODO: for an unpackage binary we don't currently copy its
    # potential local library dependencies (like those in build trees) into the
    # sandbox, and we call gdb/valgrind on the binary outside the sandbox.
    if 'Package' in report:
        for path in ('InterpreterPath', 'ExecutablePath'):
            if path in report and not os.path.exists(sandbox_dir + report[path]):
                apport.fatal('%s %s does not exist (report specified package %s)',
                             path, sandbox_dir + report[path], report['Package'])

Instead of warning with ('Cannot find package which ships %s', path) I think that should be a fatal error. It'd probably be optimal to even move the check for the crashing binary to the earliest place possible in make_sandbox.

Revision history for this message
Brian Murray (brian-murray) wrote :

Well trying to run get_file_package() before install_packages() didn't work out to well.

Traceback (most recent call last):
  File "/home/bdmurray/source-trees/apport/retracer-hack/bin/apport-retrace", line 301, in <module>
    options.dynamic_origins)
  File "/home/bdmurray/source-trees/apport/retracer-hack/apport/sandboxutils.py", line 185, in make_sandbox
    arch=report.get('Architecture'))
  File "/home/bdmurray/source-trees/apport/retracer-hack/apport/packaging_impl.py", line 463, in get_file_package
    return self._search_contents(file, map_cachedir, release, arch)
  File "/home/bdmurray/source-trees/apport/retracer-hack/apport/packaging_impl.py", line 1150, in _search_contents
    release = self._distro_release_to_codename(release)
  File "/home/bdmurray/source-trees/apport/retracer-hack/apport/packaging_impl.py", line 1133, in _distro_release_to_codename
    raise NotImplementedError('Cannot map DistroRelease to a code name without install_packages()')
NotImplementedError: Cannot map DistroRelease to a code name without install_packages()

Changed in apport (Ubuntu):
assignee: nobody → Martin Pitt (pitti)
Changed in apport (Ubuntu):
importance: Undecided → High
Revision history for this message
Martin Pitt (pitti) wrote :

> Well trying to run get_file_package() before install_packages() didn't work out to well.
> NotImplementedError: Cannot map DistroRelease to a code name without install_packages()

Indeed, as without a config directory we can't map something like "Ubuntu 15.04" to "wily", which we need to download the matching Contents.gz.

I turned the warning into a fatal, which should already help: http://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/2997 . This should be okay, as realistically the first apport.packaging.install_packages() call does not really do anything (unless you specified extra_packages), as we usually have ProcMaps available in reports and thus use the needed_runtime_packages() pass to install only the packages we need instead of all Packages: plus Dependencies:. So I think any additional potential speedup by reordering things should be miniscule, especially since (hopefully) an unknown ExecutablePath isn't the common case?

Changed in apport:
status: New → Fix Released
Changed in apport (Ubuntu):
status: New → Fix Committed
Revision history for this message
Brian Murray (brian-murray) wrote :

Somehow the error tracker is receiving plenty of crash reports from applications that aren't part of Ubuntu, so we do end up in a situation where an unknown ExecutablePath is some what common.

While the first install_packages() call doesn't do much because pkgs is empty, it stills creates a sandbox and updates the packages in it, and that seems unnecessary if pkgs is empty.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apport - 2.18.1-0ubuntu1

---------------
apport (2.18.1-0ubuntu1) wily; urgency=medium

  * New upstream bug fix release. Changes since our previous snapshot:
    - packaging.py: Only consider first word in /etc/os-release's NAME value.
      This works around Debian's inconsistent value. (LP: #1408245)
    - Unify and simplify Package: field generation in kernel_crashdump,
      kernel_oops, and package_hook by using the new Report.add_package()
      method. (LP: #1485787)
    - sandboxutils.py, make_sandbox(): Make "Cannot find package which ships
      Executable/InterpreterPath" fatal, to save some unnecessary package
      unpack cycles. (LP: #1487174)
  * etc/apport/crashdb.conf: Enable crash reports on Launchpad for wily.
    Really late, sorry about that!

 -- Martin Pitt <email address hidden> Thu, 10 Sep 2015 11:48:46 +0200

Changed in apport (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.