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: Merged
Approved by: John A Meinel
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/417881-selftest-no-apport
Merge into: lp:bzr
Diff against target: 31 lines (+10/-0)
2 files modified
NEWS (+3/-0)
bzrlib/builtins.py (+7/-0)
To merge this branch: bzr merge lp:~mbp/bzr/417881-selftest-no-apport
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+18527@code.launchpad.net

This proposal supersedes a proposal from 2010-02-02.

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

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

Revision history for this message
John A Meinel (jameinel) wrote :

I think the specific layout can get a bit into bikeshedding, but if you like the look of this layout, then lets go with it.

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

I think I'll land this without the traceback changes and we can consider that separately.

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

So I realise this has landed already, but I think its the wrong
approach ;)

Specifically, we should IMO disable apport *anyway* for development
trees - where there is a .bzr dir. That way, for selftest:
 - as packaged, we get apport bug reports about selftest (good)
 - as a dev environment, we don't get apport in our way (good)

-Rob

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

On 3 February 2010 17:18, Robert Collins <email address hidden> wrote:
> So I realise this has landed already, but I think its the wrong
> approach ;)
>
> Specifically, we should IMO disable apport *anyway* for development
> trees - where there is a .bzr dir. That way, for selftest:
>  - as packaged, we get apport bug reports about selftest (good)
>  - as a dev environment, we don't get apport in our way (good)

It's easy to change. Where do you think we should specify that
something is a development environment?

I don't think "not from a package" or "running from home" is perfect
because there are users who regularly run that way. But perhaps it is
the best tradeoff.

Alternatively we can set "debug_flags = no_apport" ourselves and
simply reverse this patch.

--
Martin <http://launchpad.net/~mbp/>

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

On Wed, 2010-02-03 at 17:42 +0000, Martin Pool wrote:
> On 3 February 2010 17:18, Robert Collins <email address hidden> wrote:
> > So I realise this has landed already, but I think its the wrong
> > approach ;)
> >
> > Specifically, we should IMO disable apport *anyway* for development
> > trees - where there is a .bzr dir. That way, for selftest:
> > - as packaged, we get apport bug reports about selftest (good)
> > - as a dev environment, we don't get apport in our way (good)
>
> It's easy to change. Where do you think we should specify that
> something is a development environment?

I was thinking if we're running from source is a good heuristic - users
that run from source and aren't up to reporting good bugs are a small
portion of our userbase, I think.

> I don't think "not from a package" or "running from home" is perfect
> because there are users who regularly run that way. But perhaps it is
> the best tradeoff.
>
> Alternatively we can set "debug_flags = no_apport" ourselves and
> simply reverse this patch.

Or we could do that instead/too. Mainly I'm saying that a user on Ubuntu
with a packaged bzr that breaks loading tests should generate an apport
problem report.

-Rob

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

So shall we perhaps pull this out and just tell developers to set -Dno_apport?

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

On Wed, 2010-02-03 at 18:21 +0000, Martin Pool wrote:
> So shall we perhaps pull this out and just tell developers to set -Dno_apport?

Yes, I think so. Perhaps a note in HACKING noting this and suggesting a
bazaar.conf stanza for it?

-Rob

Revision history for this message
Andrew Bennetts (spiv) wrote :

Martin Pool wrote:
> So shall we perhaps pull this out and just tell developers to set -Dno_apport?

FWIW, this developer already set -Dno_apport several weeks ago, so that plan
gets +1 from me :)

-Andrew.

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Andrew Bennetts wrote:
> Martin Pool wrote:
>> So shall we perhaps pull this out and just tell developers to set -Dno_apport?
>
> FWIW, this developer already set -Dno_apport several weeks ago, so that plan
> gets +1 from me :)

Me too.

Ian C.

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

reversion landed

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-03 10:06:19 +0000
3+++ NEWS 2010-02-03 15:45:25 +0000
4@@ -28,6 +28,9 @@
5 Testing
6 *******
7
8+* Don't use Apport for errors while loading or running tests.
9+ (Martin Pool, #417881)
10+
11 * Stop sending apport crash files to ``.cache`` in the directory from
12 which ``bzr selftest`` was run. (Martin Pool, #422350)
13
14
15=== modified file 'bzrlib/builtins.py'
16--- bzrlib/builtins.py 2010-01-29 10:36:23 +0000
17+++ bzrlib/builtins.py 2010-02-03 15:45:25 +0000
18@@ -3524,6 +3524,13 @@
19
20 # Make deprecation warnings visible, unless -Werror is set
21 symbol_versioning.activate_deprecation_warnings(override=False)
22+
23+ # Whatever you normally want, for the purposes of running selftest you
24+ # probably actually want to see the exception, not to turn it into an
25+ # apport failure. This is specifically turned off again for tests of
26+ # apport. We turn it off here so that eg a SyntaxError loading the
27+ # tests is caught.
28+ os.environ['APPORT_DISABLE'] = '1'
29
30 if cache_dir is not None:
31 tree_creator.TreeCreator.CACHE_ROOT = osutils.abspath(cache_dir)