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

Proposed by Martin Pool on 2010-02-02
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 2010-02-02 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.
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

5002. By Martin Pool on 2010-02-04

Revert turning off apport in cmd_selftest

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-02-02 10:38:47 +0000
+++ NEWS 2010-02-03 00:05:22 +0000
@@ -19,6 +19,21 @@
19New Features19New Features
20************20************
2121
22* If the Apport crash-reporting tool is available, bzr crashes are now
23 stored into the ``/var/crash`` apport spool directory, and the user is
24 invited to report them to the developers from there, either
25 automatically or by running ``apport-bug``. No information is sent
26 without specific permission from the user. (Martin Pool, #515052)
27
28Testing
29*******
30
31* Don't use Apport for errors while loading or running tests.
32 (Martin Pool, #417881)
33
34* Stop sending apport crash files to ``.cache`` in the directory from
35 which ``bzr selftest`` was run. (Martin Pool, #422350)
36
22bzr 2.1.0 (not released yet)37bzr 2.1.0 (not released yet)
23############################38############################
2439
2540
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2010-01-29 10:36:23 +0000
+++ bzrlib/builtins.py 2010-02-03 00:05:22 +0000
@@ -3524,6 +3524,13 @@
35243524
3525 # Make deprecation warnings visible, unless -Werror is set3525 # Make deprecation warnings visible, unless -Werror is set
3526 symbol_versioning.activate_deprecation_warnings(override=False)3526 symbol_versioning.activate_deprecation_warnings(override=False)
3527
3528 # Whatever you normally want, for the purposes of running selftest you
3529 # probably actually want to see the exception, not to turn it into an
3530 # apport failure. This is specifically turned off again for tests of
3531 # apport. We turn it off here so that eg a SyntaxError loading the
3532 # tests is caught.
3533 os.environ['APPORT_DISABLE'] = '1'
35273534
3528 if cache_dir is not None:3535 if cache_dir is not None:
3529 tree_creator.TreeCreator.CACHE_ROOT = osutils.abspath(cache_dir)3536 tree_creator.TreeCreator.CACHE_ROOT = osutils.abspath(cache_dir)
35303537
=== modified file 'bzrlib/config.py'
--- bzrlib/config.py 2009-12-17 10:01:25 +0000
+++ bzrlib/config.py 2010-02-03 00:05:22 +0000
@@ -1,4 +1,4 @@
1# Copyright (C) 2005, 2007, 2008 Canonical Ltd1# Copyright (C) 2005, 2007, 2008, 2010 Canonical Ltd
2# Authors: Robert Collins <robert.collins@canonical.com>2# Authors: Robert Collins <robert.collins@canonical.com>
3# and others3# and others
4#4#
@@ -866,12 +866,16 @@
866866
867 This doesn't implicitly create it.867 This doesn't implicitly create it.
868868
869 On Windows it's in the config directory; elsewhere in the XDG cache directory.869 On Windows it's in the config directory; elsewhere it's /var/crash
870 which may be monitored by apport. It can be overridden by
871 $APPORT_CRASH_DIR.
870 """872 """
871 if sys.platform == 'win32':873 if sys.platform == 'win32':
872 return osutils.pathjoin(config_dir(), 'Crash')874 return osutils.pathjoin(config_dir(), 'Crash')
873 else:875 else:
874 return osutils.pathjoin(xdg_cache_dir(), 'crash')876 # XXX: hardcoded in apport_python_hook.py; therefore here too -- mbp
877 # 2010-01-31
878 return os.environ.get('APPORT_CRASH_DIR', '/var/crash')
875879
876880
877def xdg_cache_dir():881def xdg_cache_dir():
878882
=== modified file 'bzrlib/crash.py'
--- bzrlib/crash.py 2009-08-20 05:47:53 +0000
+++ bzrlib/crash.py 2010-02-03 00:05:22 +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
@@ -16,10 +16,32 @@
1616
1717
18"""Handling and reporting crashes.18"""Handling and reporting crashes.
19
20A crash is an exception propagated up almost to the top level of Bazaar.
21
22If we have apport <https://launchpad.net/apport/>, we store a report of the
23crash using apport into it's /var/crash spool directory, from where the user
24can either manually send it to Launchpad. In some cases (at least Ubuntu
25development releases), Apport may pop up a window asking if they want
26to send it.
27
28Without apport, we just write a crash report to stderr and the user can report
29this manually if the wish.
30
31We never send crash data across the network without user opt-in.
32
33In principle apport can run on any platform though as of Feb 2010 there seem
34to be some portability bugs.
35
36To force this off in bzr turn set APPORT_DISBLE in the environment or
37-Dno_apport.
19"""38"""
2039
21# for interactive testing, try the 'bzr assert-fail' command 40# for interactive testing, try the 'bzr assert-fail' command
22# or see http://code.launchpad.net/~mbp/bzr/bzr-fail41# or see http://code.launchpad.net/~mbp/bzr/bzr-fail
42#
43# to test with apport it's useful to set
44# export APPORT_IGNORE_OBSOLETE_PACKAGES=1
2345
24import os46import os
25import platform47import platform
@@ -39,39 +61,42 @@
3961
4062
41def report_bug(exc_info, stderr):63def report_bug(exc_info, stderr):
42 if 'no_apport' not in debug.debug_flags:64 if ('no_apport' in debug.debug_flags) or \
43 try:65 os.environ.get('APPORT_DISABLE', None):
44 report_bug_to_apport(exc_info, stderr)66 return report_bug_legacy(exc_info, stderr)
67 try:
68 if report_bug_to_apport(exc_info, stderr):
69 # wrote a file; if None then report the old way
45 return70 return
46 except ImportError, e:71 except ImportError, e:
47 trace.mutter("couldn't find apport bug-reporting library: %s" % e)72 trace.mutter("couldn't find apport bug-reporting library: %s" % e)
48 pass73 pass
49 except Exception, e:74 except Exception, e:
50 # this should only happen if apport is installed but it didn't75 # this should only happen if apport is installed but it didn't
51 # work, eg because of an io error writing the crash file76 # work, eg because of an io error writing the crash file
52 sys.stderr.write("bzr: failed to report crash using apport:\n "77 stderr.write("bzr: failed to report crash using apport:\n "
53 " %r\n" % e)78 " %r\n" % e)
54 pass79 pass
55 report_bug_legacy(exc_info, stderr)80 return report_bug_legacy(exc_info, stderr)
5681
5782
58def report_bug_legacy(exc_info, err_file):83def report_bug_legacy(exc_info, err_file):
59 """Report a bug by just printing a message to the user."""84 """Report a bug by just printing a message to the user."""
60 trace.print_exception(exc_info, err_file)
61 err_file.write('\n')
62 err_file.write('bzr %s on python %s (%s)\n' % \85 err_file.write('bzr %s on python %s (%s)\n' % \
63 (bzrlib.__version__,86 (bzrlib.__version__,
64 bzrlib._format_version_tuple(sys.version_info),87 bzrlib._format_version_tuple(sys.version_info),
65 platform.platform(aliased=1)))88 platform.platform(aliased=1)))
89 err_file.write("plugins:\n")
90 err_file.write(_format_plugin_list())
66 err_file.write('arguments: %r\n' % sys.argv)91 err_file.write('arguments: %r\n' % sys.argv)
67 err_file.write(92 err_file.write(
68 'encoding: %r, fsenc: %r, lang: %r\n' % (93 'encoding: %r, fsenc: %r, lang: %r\n' % (
69 osutils.get_user_encoding(), sys.getfilesystemencoding(),94 osutils.get_user_encoding(), sys.getfilesystemencoding(),
70 os.environ.get('LANG')))95 os.environ.get('LANG')))
71 err_file.write("plugins:\n")96 err_file.write('\n')
72 err_file.write(_format_plugin_list())97 trace.print_exception(exc_info, err_file)
73 err_file.write(98 err_file.write(
74 "\n\n"99 "\n"
75 "*** Bazaar has encountered an internal error. This probably indicates a\n"100 "*** Bazaar has encountered an internal error. This probably indicates a\n"
76 " bug in Bazaar. You can help us fix it by filing a bug report at\n"101 " bug in Bazaar. You can help us fix it by filing a bug report at\n"
77 " https://bugs.launchpad.net/bzr/+filebug\n"102 " https://bugs.launchpad.net/bzr/+filebug\n"
@@ -81,47 +106,54 @@
81106
82def report_bug_to_apport(exc_info, stderr):107def report_bug_to_apport(exc_info, stderr):
83 """Report a bug to apport for optional automatic filing.108 """Report a bug to apport for optional automatic filing.
109
110 :returns: The name of the crash file, or None if we didn't write one.
84 """111 """
85 # this is based on apport_package_hook.py, but omitting some of the112 # this function is based on apport_package_hook.py, but omitting some of the
86 # Ubuntu-specific policy about what to report and when113 # Ubuntu-specific policy about what to report and when
87114
88 # if this fails its caught at a higher level; we don't want to open the115 # if the import fails, the exception will be caught at a higher level and
89 # crash file unless apport can be loaded.116 # we'll report the error by other means
90 import apport117 import apport
91118
92 crash_file = _open_crash_file()119 crash_filename = _write_apport_report_to_file(exc_info)
93 try:120
94 _write_apport_report_to_file(exc_info, crash_file)121 if crash_filename is None:
95 finally:122 stderr.write("\n"
96 crash_file.close()123 "apport is set to ignore crashes in this version of bzr.\n"
97124 )
98 stderr.write("bzr: ERROR: %s.%s: %s\n" 125 else:
99 "\n"126 trace.print_exception(exc_info, stderr)
100 "*** Bazaar has encountered an internal error. This probably indicates a\n"127 stderr.write("\n"
101 " bug in Bazaar. You can help us fix it by filing a bug report at\n"128 "You can report this problem to Bazaar's developers by running\n"
102 " https://bugs.launchpad.net/bzr/+filebug\n"129 " apport-bug %s\n"
103 " attaching the crash file\n"130 "if a bug-reporting window does not automatically appear.\n"
104 " %s\n"131 % (crash_filename))
105 " and including a description of the problem.\n"132 # XXX: on Windows, Mac, and other platforms where we might have the
106 "\n"133 # apport libraries but not have an apport always running, we could
107 " The crash file is plain text and you can inspect or edit it to remove\n"134 # synchronously file now
108 " private information.\n"135
109 % (exc_info[0].__module__, exc_info[0].__name__, exc_info[1],136 return crash_filename
110 crash_file.name))137
111138
112139def _write_apport_report_to_file(exc_info):
113def _write_apport_report_to_file(exc_info, crash_file):
114 import traceback140 import traceback
115 from apport.report import Report141 from apport.report import Report
116142
117 exc_type, exc_object, exc_tb = exc_info143 exc_type, exc_object, exc_tb = exc_info
118144
119 pr = Report()145 pr = Report()
120 # add_proc_info gives you the memory map of the process: this seems rarely146 # add_proc_info gets the executable and interpreter path, which is needed,
121 # useful for Bazaar and it does make the report harder to scan, though it147 # plus some less useful stuff like the memory map
122 # does tell you what binary modules are loaded.148 pr.add_proc_info()
123 # pr.add_proc_info()
124 pr.add_user_info()149 pr.add_user_info()
150
151 # Package and SourcePackage are needed so that apport will report about even
152 # non-packaged versions of bzr; also this reports on their packaged
153 # dependencies which is useful.
154 pr['SourcePackage'] = 'bzr'
155 pr['Package'] = 'bzr'
156
125 pr['CommandLine'] = pprint.pformat(sys.argv)157 pr['CommandLine'] = pprint.pformat(sys.argv)
126 pr['BzrVersion'] = bzrlib.__version__158 pr['BzrVersion'] = bzrlib.__version__
127 pr['PythonVersion'] = bzrlib._format_version_tuple(sys.version_info)159 pr['PythonVersion'] = bzrlib._format_version_tuple(sys.version_info)
@@ -137,18 +169,74 @@
137 traceback.print_exception(exc_type, exc_object, exc_tb, file=tb_file)169 traceback.print_exception(exc_type, exc_object, exc_tb, file=tb_file)
138 pr['Traceback'] = tb_file.getvalue()170 pr['Traceback'] = tb_file.getvalue()
139171
140 pr.write(crash_file)172 _attach_log_tail(pr)
173
174 # We want to use the 'bzr' crashdb so that it gets sent directly upstream,
175 # which is a reasonable default for most internal errors. However, if we
176 # set it here then apport will crash later if it doesn't know about that
177 # crashdb. Instead, we rely on the bzr package installing both a
178 # source hook telling crashes to go to this crashdb, and a crashdb
179 # configuration describing it.
180
181 # these may contain some sensitive info (smtp_passwords)
182 # TODO: strip that out and attach the rest
183 #
184 #attach_file_if_exists(report,
185 # os.path.join(dot_bzr, 'bazaar.conf', 'BzrConfig')
186 #attach_file_if_exists(report,
187 # os.path.join(dot_bzr, 'locations.conf', 'BzrLocations')
188
189 # strip username, hostname, etc
190 pr.anonymize()
191
192 if pr.check_ignored():
193 # eg configured off in ~/.apport-ignore.xml
194 return None
195 else:
196 crash_file_name, crash_file = _open_crash_file()
197 pr.write(crash_file)
198 crash_file.close()
199 return crash_file_name
200
201
202def _attach_log_tail(pr):
203 try:
204 bzr_log = open(trace._get_bzr_log_filename(), 'rt')
205 except (IOError, OSError), e:
206 pr['BzrLogTail'] = repr(e)
207 return
208 try:
209 lines = bzr_log.readlines()
210 pr['BzrLogTail'] = ''.join(lines[-40:])
211 finally:
212 bzr_log.close()
141213
142214
143def _open_crash_file():215def _open_crash_file():
144 crash_dir = config.crash_dir()216 crash_dir = config.crash_dir()
145 # user-readable only, just in case the contents are sensitive.
146 if not osutils.isdir(crash_dir):217 if not osutils.isdir(crash_dir):
147 os.makedirs(crash_dir, mode=0700)218 # on unix this should be /var/crash and should already exist; on
148 filename = 'bzr-%s-%s.crash' % (219 # Windows or if it's manually configured it might need to be created,
149 osutils.compact_date(time.time()),220 # and then it should be private
150 os.getpid(),)221 os.makedirs(crash_dir, mode=0600)
151 return open(osutils.pathjoin(crash_dir, filename), 'wt')222 date_string = time.strftime('%Y-%m-%dT%H:%M', time.gmtime())
223 # XXX: getuid doesn't work on win32, but the crash directory is per-user
224 if sys.platform == 'win32':
225 user_part = ''
226 else:
227 user_part = '.%d' % os.getuid()
228 filename = osutils.pathjoin(
229 crash_dir,
230 'bzr%s.%s.crash' % (
231 user_part,
232 date_string))
233 # be careful here that people can't play tmp-type symlink mischief in the
234 # world-writable directory
235 return filename, os.fdopen(
236 os.open(filename,
237 os.O_WRONLY|os.O_CREAT|os.O_EXCL,
238 0600),
239 'w')
152240
153241
154def _format_plugin_list():242def _format_plugin_list():
155243
=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py 2010-01-25 17:48:22 +0000
+++ bzrlib/tests/__init__.py 2010-02-03 00:05:22 +0000
@@ -1540,6 +1540,11 @@
1540 'ftp_proxy': None,1540 'ftp_proxy': None,
1541 'FTP_PROXY': None,1541 'FTP_PROXY': None,
1542 'BZR_REMOTE_PATH': None,1542 'BZR_REMOTE_PATH': None,
1543 # Generally speaking, we don't want apport reporting on crashes in
1544 # the test envirnoment unless we're specifically testing apport,
1545 # so that it doesn't leak into the real system environment. We
1546 # use an env var so it propagates to subprocesses.
1547 'APPORT_DISABLE': '1',
1543 }1548 }
1544 self.__old_env = {}1549 self.__old_env = {}
1545 self.addCleanup(self._restoreEnvironment)1550 self.addCleanup(self._restoreEnvironment)
15461551
=== modified file 'bzrlib/tests/test_crash.py'
--- bzrlib/tests/test_crash.py 2009-12-22 15:39:20 +0000
+++ bzrlib/tests/test_crash.py 2010-02-03 00:05:22 +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
@@ -18,31 +18,62 @@
18from StringIO import StringIO18from StringIO import StringIO
19import sys19import sys
2020
21
22import os
23
24
21from bzrlib import (25from bzrlib import (
26 config,
22 crash,27 crash,
23 tests,28 tests,
24 )29 osutils,
2530 )
26from bzrlib.tests import features31
2732from bzrlib.tests import (
2833 features,
29class TestApportReporting(tests.TestCase):34 TestCaseInTempDir,
35 )
36from bzrlib.tests.features import ApportFeature
37
38
39class TestApportReporting(TestCaseInTempDir):
3040
31 _test_needs_features = [features.apport]41 _test_needs_features = [features.apport]
3242
33 def test_apport_report_contents(self):43 def setUp(self):
44 TestCaseInTempDir.setUp(self)
45 self.requireFeature(ApportFeature)
46
47 def test_apport_report(self):
48 crash_dir = osutils.joinpath((self.test_base_dir, 'crash'))
49 os.mkdir(crash_dir)
50 os.environ['APPORT_CRASH_DIR'] = crash_dir
51 self.assertEquals(crash_dir, config.crash_dir())
52
53 stderr = StringIO()
54
34 try:55 try:
35 raise AssertionError("my error")56 raise AssertionError("my error")
36 except AssertionError, e:57 except AssertionError, e:
37 pass58 pass
38 outf = StringIO()59
39 crash._write_apport_report_to_file(sys.exc_info(), outf)60 crash_filename = crash.report_bug_to_apport(sys.exc_info(),
40 report = outf.getvalue()61 stderr)
4162
42 self.assertContainsRe(report, '(?m)^BzrVersion:')63 # message explaining the crash
43 # should be in the traceback64 self.assertContainsRe(stderr.getvalue(),
65 " apport-bug %s" % crash_filename)
66
67 crash_file = open(crash_filename)
68 try:
69 report = crash_file.read()
70 finally:
71 crash_file.close()
72
73 self.assertContainsRe(report,
74 '(?m)^BzrVersion:') # should be in the traceback
44 self.assertContainsRe(report, 'my error')75 self.assertContainsRe(report, 'my error')
45 self.assertContainsRe(report, 'AssertionError')76 self.assertContainsRe(report, 'AssertionError')
46 self.assertContainsRe(report, 'test_apport_report_contents')77 self.assertContainsRe(report, 'test_apport_report')
47 # should also be in there78 # should also be in there
48 self.assertContainsRe(report, '(?m)^CommandLine:')79 self.assertContainsRe(report, '(?m)^CommandLine:')
4980
=== added directory 'contrib/apport'
=== added file 'contrib/apport/README'
--- contrib/apport/README 1970-01-01 00:00:00 +0000
+++ contrib/apport/README 2010-02-03 00:05:22 +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 'contrib/apport/bzr.conf'
--- contrib/apport/bzr.conf 1970-01-01 00:00:00 +0000
+++ contrib/apport/bzr.conf 2010-02-03 00:05:22 +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 'contrib/apport/source_bzr.py'
--- contrib/apport/source_bzr.py 1970-01-01 00:00:00 +0000
+++ contrib/apport/source_bzr.py 2010-02-03 00:05:22 +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()
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