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

[Martin Pool]

[Martin Pool (mbp)]

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

[John A Meinel (jameinel)]

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

[Martin Pool (mbp)]

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

[Robert Collins (lifeless)]

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

[Martin Pool (mbp)]

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/>

[Robert Collins (lifeless)]

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

[Martin Pool (mbp)]

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

[Robert Collins (lifeless)]

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

[Andrew Bennetts (spiv)]

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.

[Ian Clatworthy (ian-clatworthy)]

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.

[Martin Pool (mbp)]

reversion landed

=== modified file 'NEWS'
--- NEWS	2010-02-03 10:06:19 +0000
+++ NEWS	2010-02-03 15:45:25 +0000
@@ -28,6 +28,9 @@
 Testing
 *******
 
+* Don't use Apport for errors while loading or running tests.
+  (Martin Pool, #417881)
+
 * Stop sending apport crash files to ``.cache`` in the directory from
   which ``bzr selftest`` was run.  (Martin Pool, #422350)
 

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-01-29 10:36:23 +0000
+++ bzrlib/builtins.py	2010-02-03 15:45:25 +0000
@@ -3524,6 +3524,13 @@
 
         # Make deprecation warnings visible, unless -Werror is set
         symbol_versioning.activate_deprecation_warnings(override=False)
+       
+        # Whatever you normally want, for the purposes of running selftest you
+        # probably actually want to see the exception, not to turn it into an
+        # apport failure.  This is specifically turned off again for tests of
+        # apport.  We turn it off here so that eg a SyntaxError loading the
+        # tests is caught.
+        os.environ['APPORT_DISABLE'] = '1'
 
         if cache_dir is not None:
             tree_creator.TreeCreator.CACHE_ROOT = osutils.abspath(cache_dir)