Bazaar

Merge lp:~mbp/bzr/2.1-528114-apport into lp:bzr/2.1

  1. 2.1-528114-apport
  2. Merge into 2.1
Proposed by Martin Pool
Status: Rejected
Rejected by: Robert Collins
Proposed branch: lp:~mbp/bzr/2.1-528114-apport
Merge into: lp:bzr/2.1
Diff against target: 243 lines (+144/-7)
9 files modified
NEWS (+36/-0)
apport/README (+13/-0)
apport/bzr-crashdb.conf (+6/-0)
apport/source_bzr.py (+54/-0)
bzr (+1/-1)
bzrlib/__init__.py (+1/-1)
bzrlib/crash.py (+18/-5)
bzrlib/tests/test_crash.py (+2/-0)
setup.py (+13/-0)
To merge this branch: bzr merge lp:~mbp/bzr/2.1-528114-apport
Reviewer Review Type Date Requested Status
James Westby Approve
Martin Pitt Pending
bzr-core Pending
Review via email: mp+22104@code.launchpad.net

Description of the change

This gets bzr to produce crash files that can be filed automatically using 'ubuntu-bug' or 'apport-cli'.

* SourcePackage, Package, ExecutablePath and InstallerPath must be included in the crash report, or apport aborts or complains
* set a CrashDb in the report so that it's filed against bzr upstream, not bzr in Ubuntu
* install a crashdb configuration and source package hook to allow the previous point to work, only when running as root

This is based off bzr 2.1 but it's probably a bit risky to land there and I would put it only into trunk in the first place.

This fixes bug 528114 and bug 391015.

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

actually this is not a complete fix; it still complains about not being able to identify the source package.

Revision history for this message
Martin Pool (mbp) wrote :

OK, so this also fixes bug 391015 by installing a source package hook and a crashdb definition. ubuntu-bug and apport-cli will now send the crash reports directly into the bzr bug tracker.

I'm not really happy with the way the apport definitions are installed from setup.py; suggestions welcome. I think it will do for now though.

This was based off 2.1 but in consideration of its riskiness and the fact that you can still file by just uploading the crash file I think it's better kept for trunk.

Revision history for this message
Martin Pool (mbp) wrote :

See https://bugs.edge.launchpad.net/apport/+bug/551330 re wanting to do without the system-wide crashdb definition.

Revision history for this message
James Westby (james-w) wrote :

Looks ok to me.

Do we want to disable the crashdb part in the Ubuntu packages?
That would give us dupe detection at the least, as when a
bug is introduced in to the Ubuntu package it may get a lot of
reports if it is easy to hit. I don't think it's important though.

Thanks,

James

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

I think at least to start with it's ok to just file them all upstream, and we should still get dupe detection there.

Thanks for the review.

Revision history for this message
Robert Collins (lifeless) wrote :

Landed in trunk, rejecting vs 2.1 as per Martin's comments

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-03-24 07:13:35 +0000
+++ NEWS 2010-03-29 08:27:29 +0000
@@ -6,6 +6,42 @@
6 :depth: 16 :depth: 1
77
88
9bzr 2.1.2
10#########
11
12:2.1.2: not released yet
13
14Compatibility Breaks
15********************
16
17New Features
18************
19
20Bug Fixes
21*********
22
23* Make sure the ``ExecutablePath`` and ``InterpreterPath`` are set in
24 Apport crash reports, to avoid "This problem report applies to a program
25 which is not installed any more" error.
26 (Martin Pool, James Westby, #528114)
27
28Improvements
29************
30
31Documentation
32*************
33
34API Changes
35***********
36
37Internals
38*********
39
40Testing
41*******
42
43
44
9bzr 2.1.145bzr 2.1.1
10#########46#########
1147
1248
=== added directory 'apport'
=== added file 'apport/README'
--- apport/README 1970-01-01 00:00:00 +0000
+++ apport/README 2010-03-29 08:27:29 +0000
@@ -0,0 +1,13 @@
1Bazaar supports semi-automatic bug reporting through Apport
2<https://launchpad.net/apport/>.
3
4If apport is not installed, an exception is printed to stderr in the usual way.
5
6For this to work properly it's suggested that two files be installed when a
7package of bzr is installed:
8
9``bzr.conf``
10 into ``/etc/apport/crashdb.conf.d``
11
12``source_bzr.py``
13 into ``/usr/share/apport/package-hooks``
014
=== added file 'apport/bzr-crashdb.conf'
--- apport/bzr-crashdb.conf 1970-01-01 00:00:00 +0000
+++ apport/bzr-crashdb.conf 2010-03-29 08:27:29 +0000
@@ -0,0 +1,6 @@
1bzr = {
2 # most bzr bugs are upstream bugs; file them there
3 'impl': 'launchpad',
4 'project': 'bzr',
5 'bug_pattern_base': 'http://people.canonical.com/~pitti/bugpatterns',
6}
07
=== added file 'apport/source_bzr.py'
--- apport/source_bzr.py 1970-01-01 00:00:00 +0000
+++ apport/source_bzr.py 2010-03-29 08:27:29 +0000
@@ -0,0 +1,54 @@
1'''apport package hook for Bazaar'''
2
3# Copyright (c) 2009, 2010 Canonical Ltd.
4# Author: Matt Zimmerman <mdz@canonical.com>
5# and others
6
7from apport.hookutils import *
8import os
9
10bzr_log = os.path.expanduser('~/.bzr.log')
11dot_bzr = os.path.expanduser('~/.bazaar')
12
13def _add_log_tail(report):
14 # may have already been added in-process
15 if 'BzrLogTail' in report:
16 return
17
18 bzr_log_lines = open(bzr_log).readlines()
19 bzr_log_lines.reverse()
20
21 bzr_log_tail = []
22 blanks = 0
23 for line in bzr_log_lines:
24 if line == '\n':
25 blanks += 1
26 bzr_log_tail.append(line)
27 if blanks >= 2:
28 break
29
30 bzr_log_tail.reverse()
31 report['BzrLogTail'] = ''.join(bzr_log_tail)
32
33
34def add_info(report):
35
36 _add_log_tail(report)
37 if 'BzrPlugins' not in report:
38 # may already be present in-process
39 report['BzrPlugins'] = command_output(['bzr', 'plugins', '-v'])
40
41 # by default assume bzr crashes are upstream bugs; this relies on
42 # having a bzr entry under /etc/apport/crashdb.conf.d/
43 report['CrashDB'] = 'bzr'
44
45 # these may contain some sensitive info (smtp_passwords)
46 # TODO: strip that out and attach the rest
47
48 #attach_file_if_exists(report,
49 # os.path.join(dot_bzr, 'bazaar.conf', 'BzrConfig')
50 #attach_file_if_exists(report,
51 # os.path.join(dot_bzr, 'locations.conf', 'BzrLocations')
52
53
54# vim: expandtab shiftwidth=4
055
=== modified file 'bzr'
--- bzr 2010-02-17 05:58:10 +0000
+++ bzr 2010-03-29 08:27:29 +0000
@@ -23,7 +23,7 @@
23import warnings23import warnings
2424
25# update this on each release25# update this on each release
26_script_version = (2, 1, 1)26_script_version = (2, 1, 2)
2727
28if __doc__ is None:28if __doc__ is None:
29 print "bzr does not support python -OO."29 print "bzr does not support python -OO."
3030
=== modified file 'bzrlib/__init__.py'
--- bzrlib/__init__.py 2010-03-24 07:13:35 +0000
+++ bzrlib/__init__.py 2010-03-29 08:27:29 +0000
@@ -44,7 +44,7 @@
44# Python version 2.0 is (2, 0, 0, 'final', 0)." Additionally we use a44# Python version 2.0 is (2, 0, 0, 'final', 0)." Additionally we use a
45# releaselevel of 'dev' for unreleased under-development code.45# releaselevel of 'dev' for unreleased under-development code.
4646
47version_info = (2, 1, 1, 'final', 0)47version_info = (2, 1, 2, 'dev', 0)
4848
49# API compatibility version: bzrlib is currently API compatible with 1.15.49# API compatibility version: bzrlib is currently API compatible with 1.15.
50api_minimum_version = (2, 1, 0)50api_minimum_version = (2, 1, 0)
5151
=== modified file 'bzrlib/crash.py'
--- bzrlib/crash.py 2009-08-20 05:47:53 +0000
+++ bzrlib/crash.py 2010-03-29 08:27:29 +0000
@@ -1,4 +1,4 @@
1# Copyright (C) 2009 Canonical Ltd1# Copyright (C) 2009, 2010 Canonical Ltd
2#2#
3# This program is free software; you can redistribute it and/or modify3# This program is free software; you can redistribute it and/or modify
4# it under the terms of the GNU General Public License as published by4# it under the terms of the GNU General Public License as published by
@@ -117,10 +117,10 @@
117 exc_type, exc_object, exc_tb = exc_info117 exc_type, exc_object, exc_tb = exc_info
118118
119 pr = Report()119 pr = Report()
120 # add_proc_info gives you the memory map of the process: this seems rarely120 # add_proc_info gives you the memory map of the process, which is not so
121 # useful for Bazaar and it does make the report harder to scan, though it121 # useful for Bazaar but does tell you what binary libraries are loaded.
122 # does tell you what binary modules are loaded.122 # More importantly it sets the ExecutablePath, InterpreterPath, etc.
123 # pr.add_proc_info()123 pr.add_proc_info()
124 pr.add_user_info()124 pr.add_user_info()
125 pr['CommandLine'] = pprint.pformat(sys.argv)125 pr['CommandLine'] = pprint.pformat(sys.argv)
126 pr['BzrVersion'] = bzrlib.__version__126 pr['BzrVersion'] = bzrlib.__version__
@@ -133,6 +133,19 @@
133 pr['PythonLoadedModules'] = _format_module_list()133 pr['PythonLoadedModules'] = _format_module_list()
134 pr['BzrDebugFlags'] = pprint.pformat(debug.debug_flags)134 pr['BzrDebugFlags'] = pprint.pformat(debug.debug_flags)
135135
136 # actually we'd rather file directly against the upstream product, but
137 # apport does seem to count on there being one in there; we might need to
138 # redirect it elsewhere anyhow
139 pr['SourcePackage'] = 'bzr'
140 pr['Package'] = 'bzr'
141
142 # tell apport to file directly against the bzr package using
143 # <https://bugs.edge.launchpad.net/bzr/+bug/391015>
144 #
145 # XXX: unfortunately apport may crash later if the crashdb definition
146 # file isn't present
147 pr['CrashDb'] = 'bzr'
148
136 tb_file = StringIO()149 tb_file = StringIO()
137 traceback.print_exception(exc_type, exc_object, exc_tb, file=tb_file)150 traceback.print_exception(exc_type, exc_object, exc_tb, file=tb_file)
138 pr['Traceback'] = tb_file.getvalue()151 pr['Traceback'] = tb_file.getvalue()
139152
=== modified file 'bzrlib/tests/test_crash.py'
--- bzrlib/tests/test_crash.py 2010-02-17 17:11:16 +0000
+++ bzrlib/tests/test_crash.py 2010-03-29 08:27:29 +0000
@@ -43,6 +43,8 @@
43 # should be in the traceback43 # should be in the traceback
44 self.assertContainsRe(report, 'my error')44 self.assertContainsRe(report, 'my error')
45 self.assertContainsRe(report, 'AssertionError')45 self.assertContainsRe(report, 'AssertionError')
46 # see https://bugs.launchpad.net/bzr/+bug/528114
47 self.assertContainsRe(report, 'ExecutablePath')
46 self.assertContainsRe(report, 'test_apport_report_contents')48 self.assertContainsRe(report, 'test_apport_report_contents')
47 # should also be in there49 # should also be in there
48 self.assertContainsRe(report, '(?m)^CommandLine:')50 self.assertContainsRe(report, '(?m)^CommandLine:')
4951
=== modified file 'setup.py'
--- setup.py 2010-03-07 13:33:37 +0000
+++ setup.py 2010-03-29 08:27:29 +0000
@@ -711,6 +711,19 @@
711 # easy_install one711 # easy_install one
712 DATA_FILES = [('man/man1', ['bzr.1'])]712 DATA_FILES = [('man/man1', ['bzr.1'])]
713713
714 if sys.platform != 'win32':
715 # see https://wiki.kubuntu.org/Apport/DeveloperHowTo
716 #
717 # checking the paths and hardcoding the check for root is a bit gross,
718 # but I don't see a cleaner way to find out the locations in a way
719 # that's going to align with the hardcoded paths in apport.
720 if os.geteuid() == 0:
721 DATA_FILES += [
722 ('/usr/share/apport/package-hooks',
723 ['apport/source_bzr.py']),
724 ('/etc/apport/crashdb.conf.d/',
725 ['apport/bzr-crashdb.conf']),]
726
714 # std setup727 # std setup
715 ARGS = {'scripts': ['bzr'],728 ARGS = {'scripts': ['bzr'],
716 'data_files': DATA_FILES,729 'data_files': DATA_FILES,

Subscribers

People subscribed via source and target branches