Code review comment for lp:~ev/apport/cache_extracted_debs

Thanks for working on this!

I would like to clean this up a little though, so that it fits better into the existing code:

9 + verbose=False, cache_dir=None, keep_unpacked=False):

How about adding a sandbox_dir=None instead? If you specify it, it uses an existing one and just unpacks the debs that it actually downloaded in this run. If it is None, it uses a temporary directory. This works exactly like the cache_dir argument. Please document the new argument in the docstring, and update the function/docstring in apport/packaging.py (the abstract base class).

Filtering by time should work OK in most circumstances, but I wonder if apt.Cache() does not have a more elegant way to ask "which debs did you actually download"?

42 + optparser.add_option('-k', '--keep-unpacked', metavar='DIR',

Likewise, I think a --sandbox-dir argument would be a bit clearer, and behave similar to --cache. --keep-unpacked sounds too much like a boolean option, specifying a directory looks strange. Please document this in man/apport-retrace.1.

This should also help to simplify the patch a bit, you won't need a separate "keep" variable, etc.

I'm quite happy to have that functionality in trunk with those changes. I originally thought you wanted to make this much more complex.

At some point we need to teach crash-digger to purge its caches and sandboxes regularly. In particular Contents.gz ages rather quickly, and the permanent sandboxes will grow pretty much indefinitely (and pile up a lot of cruft which might even get in the way). But that's a separate problem indeed.