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
1=== modified file 'NEWS'
2--- NEWS 2010-03-24 07:13:35 +0000
3+++ NEWS 2010-03-29 08:27:29 +0000
4@@ -6,6 +6,42 @@
5 :depth: 1
6
7
8+bzr 2.1.2
9+#########
10+
11+:2.1.2: not released yet
12+
13+Compatibility Breaks
14+********************
15+
16+New Features
17+************
18+
19+Bug Fixes
20+*********
21+
22+* Make sure the ``ExecutablePath`` and ``InterpreterPath`` are set in
23+ Apport crash reports, to avoid "This problem report applies to a program
24+ which is not installed any more" error.
25+ (Martin Pool, James Westby, #528114)
26+
27+Improvements
28+************
29+
30+Documentation
31+*************
32+
33+API Changes
34+***********
35+
36+Internals
37+*********
38+
39+Testing
40+*******
41+
42+
43+
44 bzr 2.1.1
45 #########
46
47
48=== added directory 'apport'
49=== added file 'apport/README'
50--- apport/README 1970-01-01 00:00:00 +0000
51+++ apport/README 2010-03-29 08:27:29 +0000
52@@ -0,0 +1,13 @@
53+Bazaar supports semi-automatic bug reporting through Apport
54+<https://launchpad.net/apport/>.
55+
56+If apport is not installed, an exception is printed to stderr in the usual way.
57+
58+For this to work properly it's suggested that two files be installed when a
59+package of bzr is installed:
60+
61+``bzr.conf``
62+ into ``/etc/apport/crashdb.conf.d``
63+
64+``source_bzr.py``
65+ into ``/usr/share/apport/package-hooks``
66
67=== added file 'apport/bzr-crashdb.conf'
68--- apport/bzr-crashdb.conf 1970-01-01 00:00:00 +0000
69+++ apport/bzr-crashdb.conf 2010-03-29 08:27:29 +0000
70@@ -0,0 +1,6 @@
71+bzr = {
72+ # most bzr bugs are upstream bugs; file them there
73+ 'impl': 'launchpad',
74+ 'project': 'bzr',
75+ 'bug_pattern_base': 'http://people.canonical.com/~pitti/bugpatterns',
76+}
77
78=== added file 'apport/source_bzr.py'
79--- apport/source_bzr.py 1970-01-01 00:00:00 +0000
80+++ apport/source_bzr.py 2010-03-29 08:27:29 +0000
81@@ -0,0 +1,54 @@
82+'''apport package hook for Bazaar'''
83+
84+# Copyright (c) 2009, 2010 Canonical Ltd.
85+# Author: Matt Zimmerman <mdz@canonical.com>
86+# and others
87+
88+from apport.hookutils import *
89+import os
90+
91+bzr_log = os.path.expanduser('~/.bzr.log')
92+dot_bzr = os.path.expanduser('~/.bazaar')
93+
94+def _add_log_tail(report):
95+ # may have already been added in-process
96+ if 'BzrLogTail' in report:
97+ return
98+
99+ bzr_log_lines = open(bzr_log).readlines()
100+ bzr_log_lines.reverse()
101+
102+ bzr_log_tail = []
103+ blanks = 0
104+ for line in bzr_log_lines:
105+ if line == '\n':
106+ blanks += 1
107+ bzr_log_tail.append(line)
108+ if blanks >= 2:
109+ break
110+
111+ bzr_log_tail.reverse()
112+ report['BzrLogTail'] = ''.join(bzr_log_tail)
113+
114+
115+def add_info(report):
116+
117+ _add_log_tail(report)
118+ if 'BzrPlugins' not in report:
119+ # may already be present in-process
120+ report['BzrPlugins'] = command_output(['bzr', 'plugins', '-v'])
121+
122+ # by default assume bzr crashes are upstream bugs; this relies on
123+ # having a bzr entry under /etc/apport/crashdb.conf.d/
124+ report['CrashDB'] = 'bzr'
125+
126+ # these may contain some sensitive info (smtp_passwords)
127+ # TODO: strip that out and attach the rest
128+
129+ #attach_file_if_exists(report,
130+ # os.path.join(dot_bzr, 'bazaar.conf', 'BzrConfig')
131+ #attach_file_if_exists(report,
132+ # os.path.join(dot_bzr, 'locations.conf', 'BzrLocations')
133+
134+
135+# vim: expandtab shiftwidth=4
136
137=== modified file 'bzr'
138--- bzr 2010-02-17 05:58:10 +0000
139+++ bzr 2010-03-29 08:27:29 +0000
140@@ -23,7 +23,7 @@
141 import warnings
142
143 # update this on each release
144-_script_version = (2, 1, 1)
145+_script_version = (2, 1, 2)
146
147 if __doc__ is None:
148 print "bzr does not support python -OO."
149
150=== modified file 'bzrlib/__init__.py'
151--- bzrlib/__init__.py 2010-03-24 07:13:35 +0000
152+++ bzrlib/__init__.py 2010-03-29 08:27:29 +0000
153@@ -44,7 +44,7 @@
154 # Python version 2.0 is (2, 0, 0, 'final', 0)." Additionally we use a
155 # releaselevel of 'dev' for unreleased under-development code.
156
157-version_info = (2, 1, 1, 'final', 0)
158+version_info = (2, 1, 2, 'dev', 0)
159
160 # API compatibility version: bzrlib is currently API compatible with 1.15.
161 api_minimum_version = (2, 1, 0)
162
163=== modified file 'bzrlib/crash.py'
164--- bzrlib/crash.py 2009-08-20 05:47:53 +0000
165+++ bzrlib/crash.py 2010-03-29 08:27:29 +0000
166@@ -1,4 +1,4 @@
167-# Copyright (C) 2009 Canonical Ltd
168+# Copyright (C) 2009, 2010 Canonical Ltd
169 #
170 # This program is free software; you can redistribute it and/or modify
171 # it under the terms of the GNU General Public License as published by
172@@ -117,10 +117,10 @@
173 exc_type, exc_object, exc_tb = exc_info
174
175 pr = Report()
176- # add_proc_info gives you the memory map of the process: this seems rarely
177- # useful for Bazaar and it does make the report harder to scan, though it
178- # does tell you what binary modules are loaded.
179- # pr.add_proc_info()
180+ # add_proc_info gives you the memory map of the process, which is not so
181+ # useful for Bazaar but does tell you what binary libraries are loaded.
182+ # More importantly it sets the ExecutablePath, InterpreterPath, etc.
183+ pr.add_proc_info()
184 pr.add_user_info()
185 pr['CommandLine'] = pprint.pformat(sys.argv)
186 pr['BzrVersion'] = bzrlib.__version__
187@@ -133,6 +133,19 @@
188 pr['PythonLoadedModules'] = _format_module_list()
189 pr['BzrDebugFlags'] = pprint.pformat(debug.debug_flags)
190
191+ # actually we'd rather file directly against the upstream product, but
192+ # apport does seem to count on there being one in there; we might need to
193+ # redirect it elsewhere anyhow
194+ pr['SourcePackage'] = 'bzr'
195+ pr['Package'] = 'bzr'
196+
197+ # tell apport to file directly against the bzr package using
198+ # <https://bugs.edge.launchpad.net/bzr/+bug/391015>
199+ #
200+ # XXX: unfortunately apport may crash later if the crashdb definition
201+ # file isn't present
202+ pr['CrashDb'] = 'bzr'
203+
204 tb_file = StringIO()
205 traceback.print_exception(exc_type, exc_object, exc_tb, file=tb_file)
206 pr['Traceback'] = tb_file.getvalue()
207
208=== modified file 'bzrlib/tests/test_crash.py'
209--- bzrlib/tests/test_crash.py 2010-02-17 17:11:16 +0000
210+++ bzrlib/tests/test_crash.py 2010-03-29 08:27:29 +0000
211@@ -43,6 +43,8 @@
212 # should be in the traceback
213 self.assertContainsRe(report, 'my error')
214 self.assertContainsRe(report, 'AssertionError')
215+ # see https://bugs.launchpad.net/bzr/+bug/528114
216+ self.assertContainsRe(report, 'ExecutablePath')
217 self.assertContainsRe(report, 'test_apport_report_contents')
218 # should also be in there
219 self.assertContainsRe(report, '(?m)^CommandLine:')
220
221=== modified file 'setup.py'
222--- setup.py 2010-03-07 13:33:37 +0000
223+++ setup.py 2010-03-29 08:27:29 +0000
224@@ -711,6 +711,19 @@
225 # easy_install one
226 DATA_FILES = [('man/man1', ['bzr.1'])]
227
228+ if sys.platform != 'win32':
229+ # see https://wiki.kubuntu.org/Apport/DeveloperHowTo
230+ #
231+ # checking the paths and hardcoding the check for root is a bit gross,
232+ # but I don't see a cleaner way to find out the locations in a way
233+ # that's going to align with the hardcoded paths in apport.
234+ if os.geteuid() == 0:
235+ DATA_FILES += [
236+ ('/usr/share/apport/package-hooks',
237+ ['apport/source_bzr.py']),
238+ ('/etc/apport/crashdb.conf.d/',
239+ ['apport/bzr-crashdb.conf']),]
240+
241 # std setup
242 ARGS = {'scripts': ['bzr'],
243 'data_files': DATA_FILES,

Subscribers

People subscribed via source and target branches