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

[Brian Murray]

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.

[Brian Murray (brian-murray)]

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()

[Martin Pitt (pitti)]

> 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?

[Brian Murray (brian-murray)]

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.

[Launchpad Janitor (janitor)]

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