Bazaar

Merge lp:~mbp/bzr/417881-selftest-no-apport into lp:bzr

  1. 417881-selftest-no-apport
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Superseded
Proposed branch: lp:~mbp/bzr/417881-selftest-no-apport
Merge into: lp:bzr
Diff against target: 534 lines (+296/-73)
9 files modified
NEWS (+15/-0)
bzrlib/builtins.py (+7/-0)
bzrlib/config.py (+7/-3)
bzrlib/crash.py (+143/-55)
bzrlib/tests/__init__.py (+5/-0)
bzrlib/tests/test_crash.py (+46/-15)
contrib/apport/README (+13/-0)
contrib/apport/bzr.conf (+6/-0)
contrib/apport/source_bzr.py (+54/-0)
To merge this branch: bzr merge lp:~mbp/bzr/417881-selftest-no-apport
Reviewer Review Type Date Requested Status
bzr-core Pending
Review via email: mp+18489@code.launchpad.net

This proposal has been superseded by a proposal from 2010-02-03.

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

Turn off apport during selftest, and put the traceback at the end where it's more useful to developers and less likely to be scrolled off by plugins.

Fixes bug 417881

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-02-02 10:38:47 +0000
3+++ NEWS 2010-02-03 00:05:22 +0000
4@@ -19,6 +19,21 @@
5 New Features
6 ************
7
8+* If the Apport crash-reporting tool is available, bzr crashes are now
9+ stored into the ``/var/crash`` apport spool directory, and the user is
10+ invited to report them to the developers from there, either
11+ automatically or by running ``apport-bug``. No information is sent
12+ without specific permission from the user. (Martin Pool, #515052)
13+
14+Testing
15+*******
16+
17+* Don't use Apport for errors while loading or running tests.
18+ (Martin Pool, #417881)
19+
20+* Stop sending apport crash files to ``.cache`` in the directory from
21+ which ``bzr selftest`` was run. (Martin Pool, #422350)
22+
23 bzr 2.1.0 (not released yet)
24 ############################
25
26
27=== modified file 'bzrlib/builtins.py'
28--- bzrlib/builtins.py 2010-01-29 10:36:23 +0000
29+++ bzrlib/builtins.py 2010-02-03 00:05:22 +0000
30@@ -3524,6 +3524,13 @@
31
32 # Make deprecation warnings visible, unless -Werror is set
33 symbol_versioning.activate_deprecation_warnings(override=False)
34+
35+ # Whatever you normally want, for the purposes of running selftest you
36+ # probably actually want to see the exception, not to turn it into an
37+ # apport failure. This is specifically turned off again for tests of
38+ # apport. We turn it off here so that eg a SyntaxError loading the
39+ # tests is caught.
40+ os.environ['APPORT_DISABLE'] = '1'
41
42 if cache_dir is not None:
43 tree_creator.TreeCreator.CACHE_ROOT = osutils.abspath(cache_dir)
44
45=== modified file 'bzrlib/config.py'
46--- bzrlib/config.py 2009-12-17 10:01:25 +0000
47+++ bzrlib/config.py 2010-02-03 00:05:22 +0000
48@@ -1,4 +1,4 @@
49-# Copyright (C) 2005, 2007, 2008 Canonical Ltd
50+# Copyright (C) 2005, 2007, 2008, 2010 Canonical Ltd
51 # Authors: Robert Collins <robert.collins@canonical.com>
52 # and others
53 #
54@@ -866,12 +866,16 @@
55
56 This doesn't implicitly create it.
57
58- On Windows it's in the config directory; elsewhere in the XDG cache directory.
59+ On Windows it's in the config directory; elsewhere it's /var/crash
60+ which may be monitored by apport. It can be overridden by
61+ $APPORT_CRASH_DIR.
62 """
63 if sys.platform == 'win32':
64 return osutils.pathjoin(config_dir(), 'Crash')
65 else:
66- return osutils.pathjoin(xdg_cache_dir(), 'crash')
67+ # XXX: hardcoded in apport_python_hook.py; therefore here too -- mbp
68+ # 2010-01-31
69+ return os.environ.get('APPORT_CRASH_DIR', '/var/crash')
70
71
72 def xdg_cache_dir():
73
74=== modified file 'bzrlib/crash.py'
75--- bzrlib/crash.py 2009-08-20 05:47:53 +0000
76+++ bzrlib/crash.py 2010-02-03 00:05:22 +0000
77@@ -1,4 +1,4 @@
78-# Copyright (C) 2009 Canonical Ltd
79+# Copyright (C) 2009, 2010 Canonical Ltd
80 #
81 # This program is free software; you can redistribute it and/or modify
82 # it under the terms of the GNU General Public License as published by
83@@ -16,10 +16,32 @@
84
85
86 """Handling and reporting crashes.
87+
88+A crash is an exception propagated up almost to the top level of Bazaar.
89+
90+If we have apport <https://launchpad.net/apport/>, we store a report of the
91+crash using apport into it's /var/crash spool directory, from where the user
92+can either manually send it to Launchpad. In some cases (at least Ubuntu
93+development releases), Apport may pop up a window asking if they want
94+to send it.
95+
96+Without apport, we just write a crash report to stderr and the user can report
97+this manually if the wish.
98+
99+We never send crash data across the network without user opt-in.
100+
101+In principle apport can run on any platform though as of Feb 2010 there seem
102+to be some portability bugs.
103+
104+To force this off in bzr turn set APPORT_DISBLE in the environment or
105+-Dno_apport.
106 """
107
108 # for interactive testing, try the 'bzr assert-fail' command
109 # or see http://code.launchpad.net/~mbp/bzr/bzr-fail
110+#
111+# to test with apport it's useful to set
112+# export APPORT_IGNORE_OBSOLETE_PACKAGES=1
113
114 import os
115 import platform
116@@ -39,39 +61,42 @@
117
118
119 def report_bug(exc_info, stderr):
120- if 'no_apport' not in debug.debug_flags:
121- try:
122- report_bug_to_apport(exc_info, stderr)
123+ if ('no_apport' in debug.debug_flags) or \
124+ os.environ.get('APPORT_DISABLE', None):
125+ return report_bug_legacy(exc_info, stderr)
126+ try:
127+ if report_bug_to_apport(exc_info, stderr):
128+ # wrote a file; if None then report the old way
129 return
130- except ImportError, e:
131- trace.mutter("couldn't find apport bug-reporting library: %s" % e)
132- pass
133- except Exception, e:
134- # this should only happen if apport is installed but it didn't
135- # work, eg because of an io error writing the crash file
136- sys.stderr.write("bzr: failed to report crash using apport:\n "
137- " %r\n" % e)
138- pass
139- report_bug_legacy(exc_info, stderr)
140+ except ImportError, e:
141+ trace.mutter("couldn't find apport bug-reporting library: %s" % e)
142+ pass
143+ except Exception, e:
144+ # this should only happen if apport is installed but it didn't
145+ # work, eg because of an io error writing the crash file
146+ stderr.write("bzr: failed to report crash using apport:\n "
147+ " %r\n" % e)
148+ pass
149+ return report_bug_legacy(exc_info, stderr)
150
151
152 def report_bug_legacy(exc_info, err_file):
153 """Report a bug by just printing a message to the user."""
154- trace.print_exception(exc_info, err_file)
155- err_file.write('\n')
156 err_file.write('bzr %s on python %s (%s)\n' % \
157 (bzrlib.__version__,
158 bzrlib._format_version_tuple(sys.version_info),
159 platform.platform(aliased=1)))
160+ err_file.write("plugins:\n")
161+ err_file.write(_format_plugin_list())
162 err_file.write('arguments: %r\n' % sys.argv)
163 err_file.write(
164 'encoding: %r, fsenc: %r, lang: %r\n' % (
165 osutils.get_user_encoding(), sys.getfilesystemencoding(),
166 os.environ.get('LANG')))
167- err_file.write("plugins:\n")
168- err_file.write(_format_plugin_list())
169+ err_file.write('\n')
170+ trace.print_exception(exc_info, err_file)
171 err_file.write(
172- "\n\n"
173+ "\n"
174 "*** Bazaar has encountered an internal error. This probably indicates a\n"
175 " bug in Bazaar. You can help us fix it by filing a bug report at\n"
176 " https://bugs.launchpad.net/bzr/+filebug\n"
177@@ -81,47 +106,54 @@
178
179 def report_bug_to_apport(exc_info, stderr):
180 """Report a bug to apport for optional automatic filing.
181+
182+ :returns: The name of the crash file, or None if we didn't write one.
183 """
184- # this is based on apport_package_hook.py, but omitting some of the
185+ # this function is based on apport_package_hook.py, but omitting some of the
186 # Ubuntu-specific policy about what to report and when
187
188- # if this fails its caught at a higher level; we don't want to open the
189- # crash file unless apport can be loaded.
190+ # if the import fails, the exception will be caught at a higher level and
191+ # we'll report the error by other means
192 import apport
193
194- crash_file = _open_crash_file()
195- try:
196- _write_apport_report_to_file(exc_info, crash_file)
197- finally:
198- crash_file.close()
199-
200- stderr.write("bzr: ERROR: %s.%s: %s\n"
201- "\n"
202- "*** Bazaar has encountered an internal error. This probably indicates a\n"
203- " bug in Bazaar. You can help us fix it by filing a bug report at\n"
204- " https://bugs.launchpad.net/bzr/+filebug\n"
205- " attaching the crash file\n"
206- " %s\n"
207- " and including a description of the problem.\n"
208- "\n"
209- " The crash file is plain text and you can inspect or edit it to remove\n"
210- " private information.\n"
211- % (exc_info[0].__module__, exc_info[0].__name__, exc_info[1],
212- crash_file.name))
213-
214-
215-def _write_apport_report_to_file(exc_info, crash_file):
216+ crash_filename = _write_apport_report_to_file(exc_info)
217+
218+ if crash_filename is None:
219+ stderr.write("\n"
220+ "apport is set to ignore crashes in this version of bzr.\n"
221+ )
222+ else:
223+ trace.print_exception(exc_info, stderr)
224+ stderr.write("\n"
225+ "You can report this problem to Bazaar's developers by running\n"
226+ " apport-bug %s\n"
227+ "if a bug-reporting window does not automatically appear.\n"
228+ % (crash_filename))
229+ # XXX: on Windows, Mac, and other platforms where we might have the
230+ # apport libraries but not have an apport always running, we could
231+ # synchronously file now
232+
233+ return crash_filename
234+
235+
236+def _write_apport_report_to_file(exc_info):
237 import traceback
238 from apport.report import Report
239
240 exc_type, exc_object, exc_tb = exc_info
241
242 pr = Report()
243- # add_proc_info gives you the memory map of the process: this seems rarely
244- # useful for Bazaar and it does make the report harder to scan, though it
245- # does tell you what binary modules are loaded.
246- # pr.add_proc_info()
247+ # add_proc_info gets the executable and interpreter path, which is needed,
248+ # plus some less useful stuff like the memory map
249+ pr.add_proc_info()
250 pr.add_user_info()
251+
252+ # Package and SourcePackage are needed so that apport will report about even
253+ # non-packaged versions of bzr; also this reports on their packaged
254+ # dependencies which is useful.
255+ pr['SourcePackage'] = 'bzr'
256+ pr['Package'] = 'bzr'
257+
258 pr['CommandLine'] = pprint.pformat(sys.argv)
259 pr['BzrVersion'] = bzrlib.__version__
260 pr['PythonVersion'] = bzrlib._format_version_tuple(sys.version_info)
261@@ -137,18 +169,74 @@
262 traceback.print_exception(exc_type, exc_object, exc_tb, file=tb_file)
263 pr['Traceback'] = tb_file.getvalue()
264
265- pr.write(crash_file)
266+ _attach_log_tail(pr)
267+
268+ # We want to use the 'bzr' crashdb so that it gets sent directly upstream,
269+ # which is a reasonable default for most internal errors. However, if we
270+ # set it here then apport will crash later if it doesn't know about that
271+ # crashdb. Instead, we rely on the bzr package installing both a
272+ # source hook telling crashes to go to this crashdb, and a crashdb
273+ # configuration describing it.
274+
275+ # these may contain some sensitive info (smtp_passwords)
276+ # TODO: strip that out and attach the rest
277+ #
278+ #attach_file_if_exists(report,
279+ # os.path.join(dot_bzr, 'bazaar.conf', 'BzrConfig')
280+ #attach_file_if_exists(report,
281+ # os.path.join(dot_bzr, 'locations.conf', 'BzrLocations')
282+
283+ # strip username, hostname, etc
284+ pr.anonymize()
285+
286+ if pr.check_ignored():
287+ # eg configured off in ~/.apport-ignore.xml
288+ return None
289+ else:
290+ crash_file_name, crash_file = _open_crash_file()
291+ pr.write(crash_file)
292+ crash_file.close()
293+ return crash_file_name
294+
295+
296+def _attach_log_tail(pr):
297+ try:
298+ bzr_log = open(trace._get_bzr_log_filename(), 'rt')
299+ except (IOError, OSError), e:
300+ pr['BzrLogTail'] = repr(e)
301+ return
302+ try:
303+ lines = bzr_log.readlines()
304+ pr['BzrLogTail'] = ''.join(lines[-40:])
305+ finally:
306+ bzr_log.close()
307
308
309 def _open_crash_file():
310 crash_dir = config.crash_dir()
311- # user-readable only, just in case the contents are sensitive.
312 if not osutils.isdir(crash_dir):
313- os.makedirs(crash_dir, mode=0700)
314- filename = 'bzr-%s-%s.crash' % (
315- osutils.compact_date(time.time()),
316- os.getpid(),)
317- return open(osutils.pathjoin(crash_dir, filename), 'wt')
318+ # on unix this should be /var/crash and should already exist; on
319+ # Windows or if it's manually configured it might need to be created,
320+ # and then it should be private
321+ os.makedirs(crash_dir, mode=0600)
322+ date_string = time.strftime('%Y-%m-%dT%H:%M', time.gmtime())
323+ # XXX: getuid doesn't work on win32, but the crash directory is per-user
324+ if sys.platform == 'win32':
325+ user_part = ''
326+ else:
327+ user_part = '.%d' % os.getuid()
328+ filename = osutils.pathjoin(
329+ crash_dir,
330+ 'bzr%s.%s.crash' % (
331+ user_part,
332+ date_string))
333+ # be careful here that people can't play tmp-type symlink mischief in the
334+ # world-writable directory
335+ return filename, os.fdopen(
336+ os.open(filename,
337+ os.O_WRONLY|os.O_CREAT|os.O_EXCL,
338+ 0600),
339+ 'w')
340
341
342 def _format_plugin_list():
343
344=== modified file 'bzrlib/tests/__init__.py'
345--- bzrlib/tests/__init__.py 2010-01-25 17:48:22 +0000
346+++ bzrlib/tests/__init__.py 2010-02-03 00:05:22 +0000
347@@ -1540,6 +1540,11 @@
348 'ftp_proxy': None,
349 'FTP_PROXY': None,
350 'BZR_REMOTE_PATH': None,
351+ # Generally speaking, we don't want apport reporting on crashes in
352+ # the test envirnoment unless we're specifically testing apport,
353+ # so that it doesn't leak into the real system environment. We
354+ # use an env var so it propagates to subprocesses.
355+ 'APPORT_DISABLE': '1',
356 }
357 self.__old_env = {}
358 self.addCleanup(self._restoreEnvironment)
359
360=== modified file 'bzrlib/tests/test_crash.py'
361--- bzrlib/tests/test_crash.py 2009-12-22 15:39:20 +0000
362+++ bzrlib/tests/test_crash.py 2010-02-03 00:05:22 +0000
363@@ -1,4 +1,4 @@
364-# Copyright (C) 2009 Canonical Ltd
365+# Copyright (C) 2009, 2010 Canonical Ltd
366 #
367 # This program is free software; you can redistribute it and/or modify
368 # it under the terms of the GNU General Public License as published by
369@@ -18,31 +18,62 @@
370 from StringIO import StringIO
371 import sys
372
373+
374+import os
375+
376+
377 from bzrlib import (
378+ config,
379 crash,
380 tests,
381- )
382-
383-from bzrlib.tests import features
384-
385-
386-class TestApportReporting(tests.TestCase):
387+ osutils,
388+ )
389+
390+from bzrlib.tests import (
391+ features,
392+ TestCaseInTempDir,
393+ )
394+from bzrlib.tests.features import ApportFeature
395+
396+
397+class TestApportReporting(TestCaseInTempDir):
398
399 _test_needs_features = [features.apport]
400
401- def test_apport_report_contents(self):
402+ def setUp(self):
403+ TestCaseInTempDir.setUp(self)
404+ self.requireFeature(ApportFeature)
405+
406+ def test_apport_report(self):
407+ crash_dir = osutils.joinpath((self.test_base_dir, 'crash'))
408+ os.mkdir(crash_dir)
409+ os.environ['APPORT_CRASH_DIR'] = crash_dir
410+ self.assertEquals(crash_dir, config.crash_dir())
411+
412+ stderr = StringIO()
413+
414 try:
415 raise AssertionError("my error")
416 except AssertionError, e:
417 pass
418- outf = StringIO()
419- crash._write_apport_report_to_file(sys.exc_info(), outf)
420- report = outf.getvalue()
421-
422- self.assertContainsRe(report, '(?m)^BzrVersion:')
423- # should be in the traceback
424+
425+ crash_filename = crash.report_bug_to_apport(sys.exc_info(),
426+ stderr)
427+
428+ # message explaining the crash
429+ self.assertContainsRe(stderr.getvalue(),
430+ " apport-bug %s" % crash_filename)
431+
432+ crash_file = open(crash_filename)
433+ try:
434+ report = crash_file.read()
435+ finally:
436+ crash_file.close()
437+
438+ self.assertContainsRe(report,
439+ '(?m)^BzrVersion:') # should be in the traceback
440 self.assertContainsRe(report, 'my error')
441 self.assertContainsRe(report, 'AssertionError')
442- self.assertContainsRe(report, 'test_apport_report_contents')
443+ self.assertContainsRe(report, 'test_apport_report')
444 # should also be in there
445 self.assertContainsRe(report, '(?m)^CommandLine:')
446
447=== added directory 'contrib/apport'
448=== added file 'contrib/apport/README'
449--- contrib/apport/README 1970-01-01 00:00:00 +0000
450+++ contrib/apport/README 2010-02-03 00:05:22 +0000
451@@ -0,0 +1,13 @@
452+Bazaar supports semi-automatic bug reporting through Apport
453+<https://launchpad.net/apport/>.
454+
455+If apport is not installed, an exception is printed to stderr in the usual way.
456+
457+For this to work properly it's suggested that two files be installed when a
458+package of bzr is installed:
459+
460+``bzr.conf``
461+ into ``/etc/apport/crashdb.conf.d``
462+
463+``source_bzr.py``
464+ into ``/usr/share/apport/package-hooks``
465
466=== added file 'contrib/apport/bzr.conf'
467--- contrib/apport/bzr.conf 1970-01-01 00:00:00 +0000
468+++ contrib/apport/bzr.conf 2010-02-03 00:05:22 +0000
469@@ -0,0 +1,6 @@
470+bzr = {
471+ # most bzr bugs are upstream bugs; file them there
472+ 'impl': 'launchpad',
473+ 'project': 'bzr',
474+ 'bug_pattern_base': 'http://people.canonical.com/~pitti/bugpatterns',
475+}
476
477=== added file 'contrib/apport/source_bzr.py'
478--- contrib/apport/source_bzr.py 1970-01-01 00:00:00 +0000
479+++ contrib/apport/source_bzr.py 2010-02-03 00:05:22 +0000
480@@ -0,0 +1,54 @@
481+'''apport package hook for Bazaar'''
482+
483+# Copyright (c) 2009, 2010 Canonical Ltd.
484+# Author: Matt Zimmerman <mdz@canonical.com>
485+# and others
486+
487+from apport.hookutils import *
488+import os
489+
490+bzr_log = os.path.expanduser('~/.bzr.log')
491+dot_bzr = os.path.expanduser('~/.bazaar')
492+
493+def _add_log_tail(report):
494+ # may have already been added in-process
495+ if 'BzrLogTail' in report:
496+ return
497+
498+ bzr_log_lines = open(bzr_log).readlines()
499+ bzr_log_lines.reverse()
500+
501+ bzr_log_tail = []
502+ blanks = 0
503+ for line in bzr_log_lines:
504+ if line == '\n':
505+ blanks += 1
506+ bzr_log_tail.append(line)
507+ if blanks >= 2:
508+ break
509+
510+ bzr_log_tail.reverse()
511+ report['BzrLogTail'] = ''.join(bzr_log_tail)
512+
513+
514+def add_info(report):
515+
516+ _add_log_tail()
517+ if 'BzrPlugins' not in report:
518+ # may already be present in-process
519+ report['BzrPlugins'] = command_output(['bzr', 'plugins', '-v'])
520+
521+ # by default assume bzr crashes are upstream bugs; this relies on
522+ # having a bzr entry under /etc/apport/crashdb.conf.d/
523+ report['CrashDB'] = 'bzr'
524+
525+ # these may contain some sensitive info (smtp_passwords)
526+ # TODO: strip that out and attach the rest
527+
528+ #attach_file_if_exists(report,
529+ # os.path.join(dot_bzr, 'bazaar.conf', 'BzrConfig')
530+ #attach_file_if_exists(report,
531+ # os.path.join(dot_bzr, 'locations.conf', 'BzrLocations')
532+
533+
534+# vim: expandtab shiftwidth=4