Merge lp:~gothicx/apport/retrace_installed_none into lp:~apport-hackers/apport/trunk

[Marco Rodrigues]

[Martin Pitt (pitti)]

http://bazaar.launchpad.net/~gothicx/apport/retrace_installed_none/revision/1595:

> hdr['Tags'] += ' ' + report['DistroRelease'].split()[0].lower + ' ' + report['DistroRelease'].split()[2]

This has some problems:

 * It will immediately crash because "lower" is a function, not a function call. This obviously hasn't been tested.
 * We should not assume that lsb_release delivers a code name on all distributions out there, so it should be tested more carefully first.
 * There is no need to add a tag for the first field. The bugs will already be filed against the "Ubuntu" distribution in Launchpad, tagging them all with "ubuntu" is just redundant.

http://bazaar.launchpad.net/~gothicx/apport/retrace_installed_none/revision/1596:

Can you please reproduce this locally with python-apt first, and perhaps put this as a reproducer into the test suite? I suppose this happens with virtual packages or something similar. Simply papering over the faulty case merely hides the underlying problem.

review: Needs Fixing

[Marco Rodrigues (gothicx)]

I remember why I've added the distro name from lsb_release to the tags. Apport currently report everything as 'ubuntu' in LP. Brian suggested to be tagged with the distro name, because we can have tags like 'ubuntu', 'kubuntu', 'xubuntu', 'gobuntu', etc.

What do you think about that ?

[Martin Pitt (pitti)]

Marco Rodrigues [2009-09-14 21:04 -0000]:
> I remember why I've added the distro name from lsb_release to the
> tags. Apport currently report everything as 'ubuntu' in LP. Brian
> suggested to be tagged with the distro name, because we can have
> tags like 'ubuntu', 'kubuntu', 'xubuntu', 'gobuntu', etc.

lsb_release doesn't give you the derivative, though. In fact it's not
that easy to know what kind of derivative you are running, since you
can install packages from any of them, and have a wild mix.

[Martin Pitt (pitti)]

No response for a long time, this has some serious bugs, and the code changed a lot since then; closing. If you still want to work on this, please open a new merge proposal. Thanks!

review: Disapprove

Unmerged revisions

1596. By Marco Rodrigues on 2009-09-12

Don't abort retrace if installed package version is None

1595. By Marco Rodrigues on 2009-09-12

Add default tags for distro id / codename

=== modified file 'apport/crashdb_impl/launchpad.py'
--- apport/crashdb_impl/launchpad.py	2009-09-04 18:48:00 +0000
+++ apport/crashdb_impl/launchpad.py	2009-09-12 12:02:56 +0000
@@ -171,7 +171,11 @@
                 hdr['Subscribers'] = 'apport'
         if report.has_key('Tags'):
             hdr['Tags'] += ' ' + report['Tags']
-
+     
+        # Add Distro ID and Codename as Tags to better LP filtering
+        # FIXME: ugly Ubuntu specific hack until LP has a real crash db
+        hdr['Tags'] += ' ' + report['DistroRelease'].split()[0].lower + ' ' + report['DistroRelease'].split()[2]
+            
         # write MIME/Multipart version into temporary file
         mime = tempfile.TemporaryFile()
         report.write_mime(mime, extra_headers=hdr)
 
=== modified file 'apport/report.py'
--- apport/report.py	2009-09-10 03:10:16 +0000
+++ apport/report.py	2009-09-12 12:02:56 +0000
@@ -217,13 +217,13 @@
         '''Add operating system information.
 
         This adds:
-        - DistroRelease: lsb_release -sir output
+        - DistroRelease: lsb_release -sirc output
         - Architecture: system architecture in distro specific notation
         - Uname: uname -srm output
         - NonfreeKernelModules: loaded kernel modules which are not free (if
             there are none, this field will not be present)
         '''
-        p = subprocess.Popen(['lsb_release', '-sir'], stdout=subprocess.PIPE,
+        p = subprocess.Popen(['lsb_release', '-sirc'], stdout=subprocess.PIPE,
             stderr=subprocess.PIPE, close_fds=True)
         self['DistroRelease'] = p.communicate()[0].strip().replace('\n', ' ')
 
 
=== modified file 'backends/packaging-apt-dpkg.py'
--- backends/packaging-apt-dpkg.py	2009-09-08 12:16:54 +0000
+++ backends/packaging-apt-dpkg.py	2009-09-12 22:55:19 +0000
@@ -189,9 +189,9 @@
 
         for line in open(sumfile):
             try:
-                # ignore lines with NUL bytes (happens, LP#96050)
+                # ignore lines with NULL bytes (happens, LP#96050)
                 if '\0' in line:
-                    print >> sys.stderr, 'WARNING:', sumfile, 'contains NUL character, ignoring line'
+                    print >> sys.stderr, 'WARNING:', sumfile, 'contains NULL character, ignoring line'
                     continue
                 words  = line.split()
                 if not words:
@@ -458,7 +458,7 @@
                 candidate_version = c[pkg].candidateVersion
             else:
                 candidate_version = c[pkg].candidate.version
-            if ver and candidate_version != ver:
+            if ver and candidate_version != ver and ver is not 'None':
                 if not pkg.endswith('-dbgsym'):
                     outdated += '%s: installed version %s, latest version: %s\n' % (
                         pkg, ver, candidate_version)