← Back to team overview

apport-hackers team mailing list archive

[Merge] lp:~brian-murray/apport/retrace-package-versions into lp:apport

 

Brian Murray has proposed merging lp:~brian-murray/apport/retrace-package-versions into lp:apport.

Requested reviews:
  Apport upstream developers (apport-hackers)

For more details, see:
https://code.launchpad.net/~brian-murray/apport/retrace-package-versions/+merge/210308

These changes to apport and its retracing process will, in some cases, attempt to install the specific package version that the client was using when they experienced the crash.

It does this by examining the report's Package and Dependencies keys to determine the version of the package used. However, this is incomplete in that the lookup for returning a package to which a file belongs (get_file_package used for items in ProcMaps) is unaware of package versions.  Given that _search_contents is searching each pocket individually we would know the pocket from which the package came, so perhaps we could do another search using those two things.  Do you have any ideas?

I've tested these changes using some crashes and corresponding coredumps I received from the error tracker.  I added some information to apport-retrace to determine how many of the packages remained without a version:

/mnt/sec-machines/20140204-cores/03273f1e-1f10-11e3-b2fe-2c768aafd08c.crash
57 packages have a version, 38 do not.
/mnt/sec-machines/20140204-cores/10165494-3e32-11e3-9577-2c768aafd08c.crash
9 packages have a version, 2 do not.
/mnt/sec-machines/20140204-cores/2e2ee904-3d70-11e3-af86-2c768aafd08c.crash
60 packages have a version, 4 do not.
/mnt/sec-machines/20140204-cores/4dd5303e-2096-11e3-921f-2c768aafd08c.crash
85 packages have a version, 20 do not.
/mnt/sec-machines/20140204-cores/586432ac-8bb7-11e3-baa3-e4115b0f8a4a.crash
115 packages have a version, 17 do not.
/mnt/sec-machines/20140204-cores/6f6484e6-145e-11e3-a19d-e4115b0f8a4a.crash
47 packages have a version, 11 do not.
/mnt/sec-machines/20140204-cores/89c60494-3ecb-11e3-afb2-e4115b0f8a4a.crash
30 packages have a version, 0 do not.
/mnt/sec-machines/20140204-cores/8e546922-8d60-11e3-a75b-e4115b0f8a4a.crash
68 packages have a version, 41 do not.
/mnt/sec-machines/20140204-cores/a4f86840-0f43-11e3-9f48-e4115b0f8a4a.crash
59 packages have a version, 48 do not.
/mnt/sec-machines/20140204-cores/b925ddf4-3d9f-11e3-a1bf-2c768aafd08c.crash
102 packages have a version, 4 do not.
/mnt/sec-machines/20140204-cores/ec88c4be-3d21-11e3-a287-2c768aafd08c.crash
1 packages have a version, 55 do not.

So it seems like there are still some improvements to be made.  I've also renamed a variable from c to cache and as c is short for continue in ipdb and this made it hard to debug cache.
-- 
https://code.launchpad.net/~brian-murray/apport/retrace-package-versions/+merge/210308
Your team Apport upstream developers is requested to review the proposed merge of lp:~brian-murray/apport/retrace-package-versions into lp:apport.
=== modified file 'apport/sandboxutils.py'
--- apport/sandboxutils.py	2013-09-30 06:07:14 +0000
+++ apport/sandboxutils.py	2014-03-10 21:46:09 +0000
@@ -74,7 +74,6 @@
     for l in libs:
         if os.path.exists(sandbox + l):
             continue
-
         pkg = apport.packaging.get_file_package(l, True, cache_dir,
                                                 release=report['DistroRelease'],
                                                 arch=report.get('Architecture'))
@@ -83,9 +82,22 @@
                 apport.log('dynamically loaded %s needs package %s, queueing' % (l, pkg))
             pkgs.add(pkg)
         else:
-                apport.warning('%s is needed, but cannot be mapped to a package', l)
-
-    return [(p, None) for p in pkgs]
+            apport.warning('%s is needed, but cannot be mapped to a package', l)
+
+    pkg_vers = {}
+    # first, grab the versions that we captured at crash time
+    for l in (report.get('Package', '') + '\n' + report.get('Dependencies', '')).splitlines():
+        if not l.strip():
+            continue
+        try:
+            (pkg, version) = l.split()[:2]
+        except ValueError:
+            apport.warning('invalid Package/Dependencies line: %s', l)
+            # invalid line, ignore
+            continue
+        pkg_vers[pkg] = version
+
+    return [(p, pkg_vers.get(p, 'None')) for p in pkgs]
 
 
 def make_sandbox(report, config_dir, cache_dir=None, sandbox_dir=None,
@@ -184,7 +196,12 @@
                                                     arch=report.get('Architecture'))
             if pkg:
                 apport.log('Installing extra package %s to get %s' % (pkg, path), log_timestamps)
-                pkgs.append((pkg, None))
+                if pkg in report['Package']:
+                    version = report['Package'].split()[1]
+                if version:
+                    pkgs.append((pkg, version))
+                else:
+                    pkgs.append((pkg, None))
             else:
                 apport.warning('Cannot find package which ships %s', path)
 

=== modified file 'backends/packaging-apt-dpkg.py'
--- backends/packaging-apt-dpkg.py	2014-01-22 08:24:55 +0000
+++ backends/packaging-apt-dpkg.py	2014-03-10 21:46:09 +0000
@@ -362,9 +362,11 @@
             match = self.__fgrep_files(file, all_lists)
 
         if match:
+            # no version here
             return os.path.splitext(os.path.basename(match))[0].split(':')[0]
 
         if uninstalled:
+            # no version here
             return self._search_contents(file, map_cachedir, release, arch)
         else:
             return None
@@ -605,15 +607,15 @@
         else:
             fetchProgress = apt.progress.base.AcquireProgress()
         if not tmp_aptroot:
-            c = self._sandbox_cache(aptroot, apt_sources, fetchProgress)
+            cache = self._sandbox_cache(aptroot, apt_sources, fetchProgress)
         else:
             self._build_apt_sandbox(aptroot, apt_sources)
-            c = apt.Cache(rootdir=os.path.abspath(aptroot))
+            cache = apt.Cache(rootdir=os.path.abspath(aptroot))
             try:
-                c.update(fetchProgress)
+                cache.update(fetchProgress)
             except apt.cache.FetchFailedException as e:
                 raise SystemError(str(e))
-            c.open()
+            cache.open()
 
         obsolete = ''
 
@@ -623,7 +625,12 @@
         real_pkgs = set()
         for (pkg, ver) in packages:
             try:
-                candidate = c[pkg].candidate
+                versions = cache[pkg].versions
+                if ver not in versions:
+                    # try the most recent if we don't have the right version
+                    candidate = cache[pkg].candidate
+                else:
+                    candidate = cache[pkg].versions[ver]
             except KeyError:
                 candidate = None
             if not candidate:
@@ -657,7 +664,7 @@
                     # Replaces/Depends, we can safely choose the first value
                     # here.
                     conflict = conflict[0]
-                    if c.is_virtual_package(conflict[0]):
+                    if cache.is_virtual_package(conflict[0]):
                         try:
                             providers = virtual_mapping[conflict[0]]
                         except KeyError:
@@ -680,32 +687,32 @@
                                 os.unlink(path)
 
             if candidate.architecture != 'all':
-                if pkg + '-dbg' in c:
+                if pkg + '-dbg' in cache:
                     real_pkgs.add(pkg + '-dbg')
                 else:
                     # install all -dbg from the source package
                     if src_records.lookup(candidate.source_name):
-                        dbgs = [p for p in src_records.binaries if p.endswith('-dbg') and p in c]
+                        dbgs = [p for p in src_records.binaries if p.endswith('-dbg') and p in cache]
                     else:
                         dbgs = []
                     if dbgs:
                         for p in dbgs:
                             real_pkgs.add(p)
                     else:
-                        if pkg + '-dbgsym' in c:
+                        if pkg + '-dbgsym' in cache:
                             real_pkgs.add(pkg + '-dbgsym')
-                            if c[pkg + '-dbgsym'].candidate.version != candidate.version:
+                            if cache[pkg + '-dbgsym'].candidate.version != candidate.version:
                                 obsolete += 'outdated debug symbol package for %s: package version %s dbgsym version %s\n' % (
-                                    pkg, candidate.version, c[pkg + '-dbgsym'].candidate.version)
+                                    pkg, candidate.version, cache[pkg + '-dbgsym'].candidate.version)
 
         for p in real_pkgs:
-            c[p].mark_install(False, False)
+            cache[p].mark_install(False, False)
 
         last_written = time.time()
         # fetch packages
         fetcher = apt.apt_pkg.Acquire(fetchProgress)
         try:
-            c.fetch_archives(fetcher=fetcher)
+            cache.fetch_archives(fetcher=fetcher)
         except apt.cache.FetchFailedException as e:
             apport.error('Package download error, try again later: %s', str(e))
             sys.exit(99)  # transient error
@@ -879,6 +886,7 @@
             out = zgrep.communicate()[0].decode('UTF-8')
             # we do not check the return code, since zgrep -m1 often errors out
             # with 'stdout: broken pipe'
+            # no version here
             if out:
                 package = out.split()[1].split(',')[0].split('/')[-1]
             if package: