Bazaar

Merge lp:~mbp/bzr/remove-logging into lp:bzr

  1. remove-logging
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Work in progress
Proposed branch: lp:~mbp/bzr/remove-logging
Merge into: lp:bzr
Diff against target: 916 lines (+197/-209)
22 files modified
NEWS (+17/-0)
bzr (+8/-3)
bzrlib/cleanup.py (+1/-1)
bzrlib/osutils.py (+1/-1)
bzrlib/status.py (+4/-6)
bzrlib/tests/TestUtil.py (+1/-20)
bzrlib/tests/__init__.py (+1/-2)
bzrlib/tests/blackbox/test_exceptions.py (+1/-18)
bzrlib/tests/blackbox/test_reconcile.py (+4/-4)
bzrlib/tests/blackbox/test_reconfigure.py (+6/-6)
bzrlib/tests/blackbox/test_tags.py (+1/-9)
bzrlib/tests/blackbox/test_upgrade.py (+11/-10)
bzrlib/tests/blackbox/test_whoami.py (+5/-4)
bzrlib/tests/per_branch/test_check.py (+3/-1)
bzrlib/tests/per_pack_repository.py (+1/-1)
bzrlib/tests/test_plugins.py (+10/-25)
bzrlib/trace.py (+39/-86)
bzrlib/transport/ssh.py (+1/-1)
bzrlib/ui/__init__.py (+4/-0)
bzrlib/ui/text.py (+15/-11)
doc/developers/index.txt (+1/-0)
doc/developers/user-interaction.txt (+62/-0)
To merge this branch: bzr merge lp:~mbp/bzr/remove-logging
Reviewer Review Type Date Requested Status
Ian Clatworthy Needs Fixing
Robert Collins (community) Needs Information
Michael Hudson-Doyle (community) Approve
Review via email: mp+15306@code.launchpad.net

This proposal supersedes a proposal from 2009-11-17.

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

This removes bzr's use of the Python logging module. Now all output either goes through bzrlib.trace to the log file (for developer-oriented information) or through Command.outf or UIFactory to the user (for regular interaction.)

This may slightly improve startup time (or enable improving it); more importantly it reduces the proliferation of different methods so that they're easier to debug and test. (Exhibit A, the diff to test_plugins, removing about 20 lines of overhead.)

This deprecates some APIs, but shouldn't break callers unless they're relying on bzrlib writing to logging. (Launchpad might?) But since we never did that very consistently, they're probably better off changing.

* Add some developer documentation about how things are meant to work - previously discussed on the list on "[rfc] developer documentation on user interaction"

* As a side-effect this causes more notice-type messages to go to stderr.

* Remove some unused test support related to logging.

* There were multiple names for equivalent methods in trace.py - some are now deprecated.

* trace.py sends user-oriented messages to the UIFactory.

Also some drive-by fixes while testing this:

* Delete test that was redundant with UpgradeRecommendedTests.

* Update some old tests from before tags were supported by default.

Revision history for this message
John A Meinel (jameinel) wrote : Posted in a previous version of this proposal
Download full text (3.5 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/remove-logging into lp:bzr.
>
> Requested reviews:
> Michael Hudson (mwhudson):
> bzr-core (bzr-core)
>
>
> This removes bzr's use of the Python logging module. Now all output either goes through bzrlib.trace to the log file (for developer-oriented information) or through Command.outf or UIFactory to the user (for regular interaction.)
>
> This may slightly improve startup time (or enable improving it); more importantly it reduces the proliferation of different methods so that they're easier to debug and test. (Exhibit A, the diff to test_plugins, removing about 20 lines of overhead.)
>
> This deprecates some APIs, but shouldn't break callers unless they're relying on bzrlib writing to logging. (Launchpad might?) But since we never did that very consistently, they're probably better off changing.
>
> * Add some developer documentation about how things are meant to work - previously discussed on the list on "[rfc] developer documentation on user interaction"
>
> * As a side-effect this causes more notice-type messages to go to stderr.
>
> * Remove some unused test support related to logging.
>
> * There were multiple names for equivalent methods in trace.py - some are now deprecated.
>
> * trace.py sends user-oriented messages to the UIFactory.
>
> Also some drive-by fixes while testing this:
>
> * Delete test that was redundant with UpgradeRecommendedTests.
>
> * Update some old tests from before tags were supported by default.
>

=== modified file 'Makefile'
- --- Makefile 2009-11-16 19:21:51 +0000
+++ Makefile 2009-11-17 08:05:40 +0000
@@ -39,7 +39,7 @@
 check: docs check-nodocs

 check-nodocs: extensions
- - $(PYTHON) -Werror -O ./bzr selftest -1v $(tests)
+ $(PYTHON) -Werror -O ./bzr selftest -1 --subunit $(tests)

^- This wasn't mentioned. Are we officially switching back? Doesn't the
pipeline need to be updated since "selftest --subunit" will no longer
return a non-0 return code? (At least from my understanding of the
conversation, Robert said you need the failure to be determined from the
later processing stages.)

Also, if we are switching to --subunit, I think it is reasonable to get
rid of -1. As you can easily parse the subunit output to just get the
failures, which means in 1 round trip you can fix everything that failed.

...

=== modified file 'NEWS'
- --- NEWS 2009-11-17 04:03:45 +0000
+++ NEWS 2009-11-17 08:05:40 +0000
@@ -418,6 +418,9 @@
 * PreviewTree behaves correctly when get_file_mtime is invoked on an
unmodified
   file. (Aaron Bentley, #251532)

+* PreviewTree behaves correctly when get_file_mtime is invoked on an
unmodified
+ file. (Aaron Bentley, #251532)
+
 * Registry objects should not use iteritems() when asked to use items().
   (Vincent Ladeuil, #430510)

^- Something doesn't look right here...

In general, all the NEWS items seem a bit confused. Are they just going
into the wrong section? (Unfortunately NEWS means you can't really use
'daggy' fixes, because then the merge into NEWS is always wrong, as it
has to be done with the "latest" version.)

Looking further, the...

Read more...

review: Needs Information
Revision history for this message
Robert Collins (lifeless) wrote : Posted in a previous version of this proposal

On Tue, 2009-11-17 at 15:27 +0000, John A Meinel wrote:
>
> check-nodocs: extensions
> - $(PYTHON) -Werror -O ./bzr selftest -1v $(tests)
> + $(PYTHON) -Werror -O ./bzr selftest -1 --subunit $(tests)

I'd do
    $(PYTHON) -Werror -O ./bzr selftest --subunit $(tests) | tee
tests.log && subunit-stats < tests.log || false

This should be robust, deal with bzr failing to start altogether as well
as individual test errors.

> ^- This wasn't mentioned. Are we officially switching back? Doesn't
> the
> pipeline need to be updated since "selftest --subunit" will no longer
> return a non-0 return code? (At least from my understanding of the
> conversation, Robert said you need the failure to be determined from
> the
> later processing stages.)
>
> Also, if we are switching to --subunit, I think it is reasonable to
> get
> rid of -1. As you can easily parse the subunit output to just get the
> failures, which means in 1 round trip you can fix everything that
> failed.

Yup. /me likes.

-Rob

Revision history for this message
Martin Pool (mbp) wrote : Posted in a previous version of this proposal

I think I have some kind of merge problem with this submission, or
perhaps I committed more than I intended to.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I don't think this will cause any great problems for Launchpad.

review: Approve
Reply
lp:~mbp/bzr/remove-logging updated
4838. By Martin Pool

Correction to error handling test that depended on python logging behaviour

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

You seem to be deleted some test coverage (the FormatDeprecationWarning test).

I like the concept of removing logging.

However, the changes for what goes to stdout/stderr are a bit mixed.

For reconcile, the actions taken were intended to be the output when I wrote it, I'm not entirely happy with that going to stderr.

For reconfigure I'm pretty sure Aaron would consider that the output of the command similarly.

For upgrade, the warning that a particular checkout won't be upgraded is something I consider part of the proper output of the command.

I guess is tied to my disagreeing with :
915 +We normally reserve stdout for bulk output data, such as diffs, logs, or
916 +bundles, and put all UIFactory output onto stderr.

I think that the *intended output* should go to stdout, and for some commands that is bulk data, for other commands its not.

As it stands I really am not comfortable with this landing, I think it will make our UI worse, for all that it does make a bunch of internal stuff cleaner.

review: Needs Information
Reply
Revision history for this message
Robert Collins (lifeless) wrote :

Please excuse my fail typing - 'deleting' not 'deleted' and 'guess this is tied' not 'guess is tied'.

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

See also https://bugs.edge.launchpad.net/bzr/+bug/317779

@lifeless I'll try to redraft this to not change any blackbox behaviour.

The FormatDeprecationWarning test was strictly redundant with another test.

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

On Wed, 2009-12-02 at 01:58 +0000, Martin Pool wrote:
> See also https://bugs.edge.launchpad.net/bzr/+bug/317779
>
> @lifeless I'll try to redraft this to not change any blackbox behaviour.

Thanks, muchly appreciated.

> The FormatDeprecationWarning test was strictly redundant with another test.

oh cool.

Rob

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Wed, 2009-12-02 at 01:58 +0000, Martin Pool wrote:
>> See also https://bugs.edge.launchpad.net/bzr/+bug/317779
>>
>> @lifeless I'll try to redraft this to not change any blackbox behaviour.
>
> Thanks, muchly appreciated.
>
>> The FormatDeprecationWarning test was strictly redundant with another test.
>
> oh cool.
>
> Rob
>

I should note that paramiko makes extensive use of the python logging
infrastructure. If we don't set up a default logging setup then paramiko
trying to log stuff will print out "unable to get default logging" or
whatever that error message is.

I haven't looked if Martin's changes actually effect this, but I figured
I'd mention it.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksWnrIACgkQJdeBCYSNAAPQxQCgtdbSaEyaIbMLiPl/L7LDO3no
h1kAoJ2zB5w9b0MOxWgY/CasDKHqZ22+
=u1LZ
-----END PGP SIGNATURE-----

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

2009/12/3 John Arbash Meinel <email address hidden>:
> I should note that paramiko makes extensive use of the python logging
> infrastructure. If we don't set up a default logging setup then paramiko
> trying to log stuff will print out "unable to get default logging" or
> whatever that error message is.

Actually I think it works _better_ if we don't register a default
logger, oddly enough. But I will check it again by forcing the use of
paramiko.

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

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

On Wed, 2009-12-02 at 22:00 +0000, Martin Pool wrote:
> 2009/12/3 John Arbash Meinel <email address hidden>:
> > I should note that paramiko makes extensive use of the python logging
> > infrastructure. If we don't set up a default logging setup then paramiko
> > trying to log stuff will print out "unable to get default logging" or
> > whatever that error message is.
>
> Actually I think it works _better_ if we don't register a default
> logger, oddly enough. But I will check it again by forcing the use of
> paramiko.

It will say to console 'could not find logger for ...', pqm runs into
this all the time.

We do need to setup logging for paramiko :(.

-Rob

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

A bunch of minor things in trace.py need tweaking (see below) but my main concern is --quiet support. IIUIC, note messages are no longer suppressed if --quiet is passed which is wrong. A simple if test added to note() ought to fix it?

trace.py things ...

1. no need to import itself

2. enable_default_logging() stuff:
   * docstring refers to _push_log_file, not push_log_file
   * result variable set but never used

3. the API change to pop_log_file() ought to be mentioned in NEWS

Otherwise, this looks good. Well done!

review: Needs Fixing
Reply
Revision history for this message
Martin Pool (mbp) wrote :

> IIUIC, note messages are no longer suppressed if --quiet is passed which is wrong. A simple if test added to note() ought to fix it?

This is in fact broken; test added.

=== modified file 'bzrlib/tests/test_trace.py'
--- bzrlib/tests/test_trace.py 2009-11-08 19:07:00 +0000
+++ bzrlib/tests/test_trace.py 2009-12-10 05:43:18 +0000
@@ -27,6 +27,8 @@

 from bzrlib import (
     errors,
+ trace,
+ ui,
     )
 from bzrlib.tests import TestCaseInTempDir, TestCase
 from bzrlib.trace import (
@@ -263,8 +265,19 @@
         # Confirm the old API still works
         be_quiet(True)
         self.assertEqual(-1, get_verbosity_level())
+
+ out = StringIO()
+ err = StringIO()
+ ui.ui_factory = ui.text.TextUIFactory(StringIO(''), out, err)
+ trace.note("sample note")
+ self.assertEqual('', out.getvalue())
+ self.assertEqual('', err.getvalue())
+
         be_quiet(False)
         self.assertEqual(0, get_verbosity_level())
+ trace.note("sample note")
+ self.assertEqual('', out.getvalue())
+ self.assertEqual('bzr: sample note', err.getvalue())

 class TestBzrLog(TestCaseInTempDir):

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
>> IIUIC, note messages are no longer suppressed if --quiet is passed which is wrong. A simple if test added to note() ought to fix it?
>
> This is in fact broken; test added.
>
>
> === modified file 'bzrlib/tests/test_trace.py'
> --- bzrlib/tests/test_trace.py 2009-11-08 19:07:00 +0000
> +++ bzrlib/tests/test_trace.py 2009-12-10 05:43:18 +0000
> @@ -27,6 +27,8 @@
>
> from bzrlib import (
> errors,
> + trace,
> + ui,
> )
> from bzrlib.tests import TestCaseInTempDir, TestCase
> from bzrlib.trace import (
> @@ -263,8 +265,19 @@
> # Confirm the old API still works
> be_quiet(True)
> self.assertEqual(-1, get_verbosity_level())
> +
> + out = StringIO()
> + err = StringIO()
> + ui.ui_factory = ui.text.TextUIFactory(StringIO(''), out, err)
> + trace.note("sample note")
> + self.assertEqual('', out.getvalue())
> + self.assertEqual('', err.getvalue())
> +
> be_quiet(False)
> self.assertEqual(0, get_verbosity_level())
> + trace.note("sample note")
> + self.assertEqual('', out.getvalue())
> + self.assertEqual('bzr: sample note', err.getvalue())
>
>
> class TestBzrLog(TestCaseInTempDir):
>
>

The test looks reasonable, though this doesn't show the fix :).

Also, it seems a bit of a shame not to have these isolated into separate
tests.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksgkKIACgkQJdeBCYSNAANulACdFGsbMowXmSC93j537PBhzY9U
BlMAoMU0Od7MXnSYzkMuIgx139hcCUUp
=qJdX
-----END PGP SIGNATURE-----

Reply

Unmerged revisions

4838. By Martin Pool

Correction to error handling test that depended on python logging behaviour

4837. By Martin Pool

Remove redundant 'warning' prefix from some messages

4836. By Martin Pool

Cope when sys.exitfunc doesn't exist

4835. By Martin Pool

Remove use of python logging, and show errors through UIFactory instead.

As a side effect, this sends some error-type messages to stderr instead of
stdout.

Remove obsolete bzrlib.tests.TestUtil.LogCollector and makeCollectingLogger.

4834. By Martin Pool

Document bzrlib.tests.TestUIFactory

4833. By Martin Pool

Tags can be tested in the default format now

4832. By Martin Pool

Add user interaction architecture doc

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-11-26 17:23:54 +0000
3+++ NEWS 2009-11-27 06:20:25 +0000
4@@ -14,6 +14,10 @@
5 Compatibility Breaks
6 ********************
7
8+* Some messages that were previously sent to stdout now go to stderr, as
9+ part of making handling of such things more consistent across bzr.
10+ (Martin Pool)
11+
12 * The BZR_SSH environmental variable may now be set to the path of a secure
13 shell client. If currently set to the value ``ssh`` it will now guess the
14 vendor of the program with that name, to restore the old behaviour that
15@@ -40,6 +44,9 @@
16 * ``bzr serve`` is more clear about the risk of supplying --allow-writes.
17 (Robert Collins, #84659)
18
19+* Cope if sys.exitfunc doesn't exist because no exitfuncs have been
20+ registered. (Martin Pool)
21+
22 * Lots of bugfixes for the test suite on Windows. We should once again
23 have a test suite with no failures on Windows. (John Arbash Meinel)
24
25@@ -64,6 +71,9 @@
26 API Changes
27 ***********
28
29+* bzrlib no longer uses ``logging`` for messages, just its own UIFactory.
30+ (Martin Pool)
31+
32 * ``bzrlib.textui`` (vestigial module) removed. (Martin Pool)
33
34 Internals
35@@ -513,6 +523,13 @@
36 ``diff.get_trees_and_branches_to_diff``. It is now a public API, and it
37 returns the old and new branches. (Gary van der Merwe)
38
39+* ``bzrlib.tests.TestUtil.LogCollector`` and ``makeCollectingLogger``
40+ have been deleted. bzrlib no longer uses ``logging`` so tests should
41+ examine output using a UIFactory or ``run_bzr``. (Martin Pool)
42+
43+* ``TextUIFactory`` constructor parameters providing file handles are now
44+ mandatory. (Martin Pool)
45+
46 * ``bzrlib.trace.log_error``, ``error`` and ``info`` have been deprecated.
47 (Martin Pool)
48
49
50=== modified file 'bzr'
51--- bzr 2009-10-30 16:13:05 +0000
52+++ bzr 2009-11-27 06:20:25 +0000
53@@ -144,9 +144,14 @@
54 if profiling:
55 profile_imports.log_stack_info(sys.stderr)
56
57- # run anything registered by atexit, because it won't be run in the normal
58- # way
59- sys.exitfunc()
60+ # Run anything registered by atexit, because it won't be run in the normal
61+ # way; note that this interface is deprecated and only going to be present
62+ # if somebody has already registered something. We could alternatively
63+ # use atexit._run_exitfuncs but that's undocumented and requires loading
64+ # another module. -- mbp 200911127
65+ exitfunc = getattr(sys, 'exitfunc', None)
66+ if exitfunc:
67+ exitfunc()
68
69 # By this point we really have completed everything we want to do, and
70 # there's no point doing any additional cleanup. Abruptly exiting here
71
72=== modified file 'bzrlib/cleanup.py'
73--- bzrlib/cleanup.py 2009-10-26 06:23:14 +0000
74+++ bzrlib/cleanup.py 2009-11-27 06:20:25 +0000
75@@ -53,7 +53,7 @@
76 trace.mutter('Cleanup failed:')
77 trace.log_exception_quietly()
78 if 'cleanup' in debug.debug_flags:
79- trace.warning('bzr: warning: Cleanup failed: %s', exc)
80+ trace.warning('Cleanup failed: %s', exc)
81
82
83 def _run_cleanup(func, *args, **kwargs):
84
85=== modified file 'bzrlib/osutils.py'
86--- bzrlib/osutils.py 2009-11-10 07:01:56 +0000
87+++ bzrlib/osutils.py 2009-11-27 06:20:25 +0000
88@@ -929,7 +929,7 @@
89 # the warnings framework should by default show this only once
90 from bzrlib.trace import warning
91 warning(
92- "bzr: warning: some compiled extensions could not be loaded; "
93+ "some compiled extensions could not be loaded; "
94 "see <https://answers.launchpad.net/bzr/+faq/703>")
95 # we no longer show the specific missing extensions here, because it makes
96 # the message too long and scary - see
97
98=== modified file 'bzrlib/status.py'
99--- bzrlib/status.py 2009-08-06 05:07:12 +0000
100+++ bzrlib/status.py 2009-11-27 06:20:25 +0000
101@@ -1,4 +1,4 @@
102-# Copyright (C) 2005, 2006, 2007 Canonical Ltd
103+# Copyright (C) 2005, 2006, 2007, 2009 Canonical Ltd
104 #
105 # This program is free software; you can redistribute it and/or modify
106 # it under the terms of the GNU General Public License as published by
107@@ -20,6 +20,7 @@
108 delta as _mod_delta,
109 log,
110 osutils,
111+ trace,
112 tree,
113 tsort,
114 revision as _mod_revision,
115@@ -28,10 +29,7 @@
116 from bzrlib.osutils import is_inside_any
117 from bzrlib.symbol_versioning import (deprecated_function,
118 )
119-from bzrlib.trace import mutter, warning
120-
121-# TODO: when showing single-line logs, truncate to the width of the terminal
122-# if known, but only if really going to the terminal (not into a file)
123+from bzrlib.trace import mutter
124
125
126 def show_tree_status(wt, show_unchanged=None,
127@@ -84,7 +82,7 @@
128 new_is_working_tree = True
129 if revision is None:
130 if wt.last_revision() != wt.branch.last_revision():
131- warning("working tree is out of date, run 'bzr update'")
132+ trace.note("working tree is out of date, run 'bzr update'")
133 new = wt
134 old = new.basis_tree()
135 elif len(revision) > 0:
136
137=== modified file 'bzrlib/tests/TestUtil.py'
138--- bzrlib/tests/TestUtil.py 2009-03-23 14:59:43 +0000
139+++ bzrlib/tests/TestUtil.py 2009-11-27 06:20:25 +0000
140@@ -1,4 +1,4 @@
141-# Copyright (C) 2004, 2005, 2006 Canonical Ltd
142+# Copyright (C) 2004, 2005, 2006, 2009 Canonical Ltd
143 # Author: Robert Collins <robert.collins@canonical.com>
144 #
145 # This program is free software; you can redistribute it and/or modify
146@@ -17,7 +17,6 @@
147 #
148
149 import sys
150-import logging
151 import unittest
152
153 # Mark this python module as being part of the implementation
154@@ -26,24 +25,6 @@
155 __unittest = 1
156
157
158-class LogCollector(logging.Handler):
159- def __init__(self):
160- logging.Handler.__init__(self)
161- self.records=[]
162- def emit(self, record):
163- self.records.append(record.getMessage())
164-
165-
166-def makeCollectingLogger():
167- """I make a logger instance that collects its logs for programmatic analysis
168- -> (logger, collector)"""
169- logger=logging.Logger("collector")
170- handler=LogCollector()
171- handler.setFormatter(logging.Formatter("%(levelname)s: %(message)s"))
172- logger.addHandler(handler)
173- return logger, handler
174-
175-
176 def visitTests(suite, visitor):
177 """A foreign method for visiting the tests in a test suite."""
178 for test in suite._tests:
179
180=== modified file 'bzrlib/tests/__init__.py'
181--- bzrlib/tests/__init__.py 2009-11-24 08:28:41 +0000
182+++ bzrlib/tests/__init__.py 2009-11-27 06:20:25 +0000
183@@ -1880,8 +1880,7 @@
184 overall behavior of the bzr application (rather than a unit test
185 or a functional test of the library.)
186
187- This sends the stdout/stderr results into the test's log,
188- where it may be useful for debugging. See also run_captured.
189+ :returns: (out, err) tuple of strings of output.
190
191 :keyword stdin: A string to be used as stdin for the command.
192 :keyword retcode: The status code the command should return;
193
194=== modified file 'bzrlib/tests/blackbox/test_exceptions.py'
195--- bzrlib/tests/blackbox/test_exceptions.py 2009-08-20 06:25:02 +0000
196+++ bzrlib/tests/blackbox/test_exceptions.py 2009-11-27 06:20:25 +0000
197@@ -1,4 +1,4 @@
198-# Copyright (C) 2006, 2007 Canonical Ltd
199+# Copyright (C) 2006, 2007, 2009 Canonical Ltd
200 #
201 # This program is free software; you can redistribute it and/or modify
202 # it under the terms of the GNU General Public License as published by
203@@ -43,20 +43,3 @@
204 self.assertContainsRe(err,
205 r'exceptions\.AssertionError: always fails\n')
206 self.assertContainsRe(err, r'Bazaar has encountered an internal error')
207-
208-
209-class TestDeprecationWarning(TestCaseInTempDir):
210-
211- def test_repository_deprecation_warning(self):
212- """Old formats give a warning"""
213- # the warning's normally off for testing but we reenable it
214- repository._deprecation_warning_done = False
215- try:
216- os.mkdir('foo')
217- bzrdir.BzrDirFormat5().initialize('foo')
218- out, err = self.run_bzr("status foo")
219- self.assertContainsRe(self._get_log(keep_log_file=True),
220- "bzr upgrade")
221- finally:
222- repository._deprecation_warning_done = True
223-
224
225=== modified file 'bzrlib/tests/blackbox/test_reconcile.py'
226--- bzrlib/tests/blackbox/test_reconcile.py 2009-03-23 14:59:43 +0000
227+++ bzrlib/tests/blackbox/test_reconcile.py 2009-11-27 06:20:25 +0000
228@@ -46,7 +46,7 @@
229 does_backup_text = "Inventory ok.\n"
230 else:
231 does_backup_text = ""
232- self.assertEqualDiff(out, "Reconciling branch %s\n"
233+ self.assertEqualDiff(err, "Reconciling branch %s\n"
234 "revision_history ok.\n"
235 "Reconciling repository %s\n"
236 "%s"
237@@ -54,7 +54,7 @@
238 (t.branch.base,
239 t.bzrdir.root_transport.base,
240 does_backup_text))
241- self.assertEqualDiff(err, "")
242+ self.assertEqualDiff(out, "")
243
244 def test_does_something_reconcile(self):
245 t = bzrdir.BzrDir.create_standalone_workingtree('.')
246@@ -82,5 +82,5 @@
247 (t.branch.base,
248 t.bzrdir.root_transport.base,
249 does_backup_text))
250- self.assertEqualDiff(expected, out)
251- self.assertEqualDiff(err, "")
252+ self.assertEqualDiff(expected, err)
253+ self.assertEqualDiff("", out)
254
255=== modified file 'bzrlib/tests/blackbox/test_reconfigure.py'
256--- bzrlib/tests/blackbox/test_reconfigure.py 2009-07-10 08:18:28 +0000
257+++ bzrlib/tests/blackbox/test_reconfigure.py 2009-11-27 06:20:25 +0000
258@@ -211,16 +211,16 @@
259 branch_2 = tree_2.branch
260 # now reconfigure to be stacked
261 out, err = self.run_bzr('reconfigure --stacked-on b1 b2')
262- self.assertContainsRe(out,
263+ self.assertContainsRe(err,
264 '^.*/b2/ is now stacked on ../b1\n$')
265- self.assertEquals('', err)
266+ self.assertEquals('', out)
267 # can also give the absolute URL of the branch, and it gets stored
268 # as a relative path if possible
269 out, err = self.run_bzr('reconfigure --stacked-on %s b2'
270 % (self.get_url('b1'),))
271- self.assertContainsRe(out,
272+ self.assertContainsRe(err,
273 '^.*/b2/ is now stacked on ../b1\n$')
274- self.assertEquals('', err)
275+ self.assertEquals('', out)
276 # It should be given a relative URL to the destination, if possible,
277 # because that's most likely to work across different transports
278 self.assertEquals(branch_2.get_stacked_on_url(),
279@@ -230,9 +230,9 @@
280 tree_2.commit('update foo')
281 # Now turn it off again
282 out, err = self.run_bzr('reconfigure --unstacked b2')
283- self.assertContainsRe(out,
284+ self.assertContainsRe(err,
285 '^.*/b2/ is now not stacked\n$')
286- self.assertEquals('', err)
287+ self.assertEquals('', out)
288 self.assertRaises(errors.NotStacked,
289 branch_2.get_stacked_on_url)
290
291
292=== modified file 'bzrlib/tests/blackbox/test_tags.py'
293--- bzrlib/tests/blackbox/test_tags.py 2009-08-20 04:09:58 +0000
294+++ bzrlib/tests/blackbox/test_tags.py 2009-11-27 06:20:25 +0000
295@@ -1,4 +1,4 @@
296-# Copyright (C) 2007 Canonical Ltd
297+# Copyright (C) 2007, 2009 Canonical Ltd
298 #
299 # This program is free software; you can redistribute it and/or modify
300 # it under the terms of the GNU General Public License as published by
301@@ -30,14 +30,6 @@
302
303 class TestTagging(TestCaseWithTransport):
304
305- # as of 0.14, the default format doesn't do tags so we need to use a
306- # specific format
307-
308- def make_branch_and_tree(self, relpath):
309- format = bzrdir.format_registry.make_bzrdir('dirstate-with-subtree')
310- return TestCaseWithTransport.make_branch_and_tree(self, relpath,
311- format=format)
312-
313 def test_tag_command_help(self):
314 out, err = self.run_bzr('help tag')
315 self.assertContainsRe(out, 'Create, remove or modify a tag')
316
317=== modified file 'bzrlib/tests/blackbox/test_upgrade.py'
318--- bzrlib/tests/blackbox/test_upgrade.py 2009-08-21 02:10:06 +0000
319+++ bzrlib/tests/blackbox/test_upgrade.py 2009-11-27 06:20:25 +0000
320@@ -86,11 +86,12 @@
321 (out, err) = self.run_bzr('upgrade current_format_checkout', retcode=3)
322 self.assertEqual("This is a checkout. The branch (%s) needs to be "
323 "upgraded separately.\n"
324+ "bzr: ERROR: The branch format Meta "
325+ "directory format 1 is already at the most "
326+ "recent format.\n"
327 % get_transport(self.get_url('current_format_branch')).base,
328- out)
329- self.assertEqualDiff("bzr: ERROR: The branch format Meta "
330- "directory format 1 is already at the most "
331- "recent format.\n", err)
332+ err)
333+ self.assertEqualDiff('', out)
334
335 def test_upgrade_checkout(self):
336 # upgrading a checkout should work
337@@ -121,8 +122,8 @@
338 adding prefixes to revision-store
339 starting upgrade from format 6 to metadir
340 finished
341-""" % (url, url, url), out)
342- self.assertEqualDiff("", err)
343+""" % (url, url, url), err)
344+ self.assertEqualDiff("", out)
345 self.assertTrue(isinstance(
346 bzrdir.BzrDir.open(self.get_url('format_5_branch'))._format,
347 bzrdir.BzrDirMetaFormat1))
348@@ -142,8 +143,8 @@
349 starting repository conversion
350 repository converted
351 finished
352-""" % (url, url, url), out)
353- self.assertEqualDiff("", err)
354+""" % (url, url, url), err)
355+ self.assertEqualDiff("", out)
356 converted_dir = bzrdir.BzrDir.open(self.get_url('metadir_weave_branch'))
357 self.assertTrue(isinstance(converted_dir._format,
358 bzrdir.BzrDirMetaFormat1))
359@@ -180,8 +181,8 @@
360 starting repository conversion
361 repository converted
362 finished
363-""" % (url, url, url), out)
364- self.assertEqual('', err)
365+""" % (url, url, url), err)
366+ self.assertEqual('', out)
367
368
369 class UpgradeRecommendedTests(TestCaseWithTransport):
370
371=== modified file 'bzrlib/tests/blackbox/test_whoami.py'
372--- bzrlib/tests/blackbox/test_whoami.py 2009-03-23 14:59:43 +0000
373+++ bzrlib/tests/blackbox/test_whoami.py 2009-11-27 06:20:25 +0000
374@@ -1,4 +1,4 @@
375-# Copyright (C) 2006 Canonical Ltd
376+# Copyright (C) 2006, 2009 Canonical Ltd
377 #
378 # This program is free software; you can redistribute it and/or modify
379 # it under the terms of the GNU General Public License as published by
380@@ -108,6 +108,7 @@
381 """verify that a warning is displayed if no email is given."""
382 self.make_branch_and_tree('.')
383 display = self.run_bzr(['whoami', 'Branch Identity'])[1]
384- self.assertEquals('"Branch Identity" does not seem to contain an '
385- 'email address. This is allowed, but not '
386- 'recommended.\n', display)
387+ self.assertEquals('bzr: warning: '
388+ '"Branch Identity" does not seem to contain an '
389+ 'email address. This is allowed, but not '
390+ 'recommended.\n', display)
391
392=== modified file 'bzrlib/tests/per_branch/test_check.py'
393--- bzrlib/tests/per_branch/test_check.py 2009-08-04 04:36:34 +0000
394+++ bzrlib/tests/per_branch/test_check.py 2009-11-27 06:20:25 +0000
395@@ -60,7 +60,9 @@
396 self.addCleanup(tree.unlock)
397 refs = self.make_refs(tree.branch)
398 result = tree.branch.check(refs)
399- ui.ui_factory = tests.TestUIFactory(stdout=StringIO())
400+ ui.ui_factory = tests.TestUIFactory(stdin='',
401+ stdout=StringIO(),
402+ stderr=StringIO())
403 result.report_results(True)
404 self.assertContainsRe('revno does not match len',
405 ui.ui_factory.stdout.getvalue())
406
407=== modified file 'bzrlib/tests/per_pack_repository.py'
408--- bzrlib/tests/per_pack_repository.py 2009-09-07 03:35:06 +0000
409+++ bzrlib/tests/per_pack_repository.py 2009-11-27 06:20:25 +0000
410@@ -689,7 +689,7 @@
411 # But it does log an error
412 log_file = self._get_log(keep_log_file=True)
413 self.assertContainsRe(log_file, 'abort_write_group failed')
414- self.assertContainsRe(log_file, r'INFO bzr: ERROR \(ignored\):')
415+ self.assertContainsRe(log_file, 'NoSuchFile') # the probable error cause
416 if token is not None:
417 repo.leave_lock_in_place()
418
419
420=== modified file 'bzrlib/tests/test_plugins.py'
421--- bzrlib/tests/test_plugins.py 2009-09-18 14:53:37 +0000
422+++ bzrlib/tests/test_plugins.py 2009-11-27 06:20:25 +0000
423@@ -1,4 +1,4 @@
424-# Copyright (C) 2005, 2007 Canonical Ltd
425+# Copyright (C) 2005, 2007, 2009 Canonical Ltd
426 #
427 # This program is free software; you can redistribute it and/or modify
428 # it under the terms of the GNU General Public License as published by
429@@ -16,11 +16,6 @@
430
431 """Tests for plugins"""
432
433-# XXX: There are no plugin tests at the moment because the plugin module
434-# affects the global state of the process. See bzrlib/plugins.py for more
435-# comments.
436-
437-import logging
438 import os
439 from StringIO import StringIO
440 import sys
441@@ -30,6 +25,7 @@
442 osutils,
443 plugin,
444 tests,
445+ ui,
446 )
447 import bzrlib.plugin
448 import bzrlib.plugins
449@@ -202,28 +198,17 @@
450 :param name: The name of the plugin.
451 :return: A string with the log from the plugin loading call.
452 """
453- # Capture output
454 stream = StringIO()
455+ ui.ui_factory = tests.TestUIFactory(stdout=stream, stderr=stream,
456+ stdin=None)
457 try:
458- handler = logging.StreamHandler(stream)
459- log = logging.getLogger('bzr')
460- log.addHandler(handler)
461- try:
462- try:
463- bzrlib.plugin.load_from_path(['.'])
464- finally:
465- if 'bzrlib.plugins.%s' % name in sys.modules:
466- del sys.modules['bzrlib.plugins.%s' % name]
467- if getattr(bzrlib.plugins, name, None):
468- delattr(bzrlib.plugins, name)
469- finally:
470- # Stop capturing output
471- handler.flush()
472- handler.close()
473- log.removeHandler(handler)
474- return stream.getvalue()
475+ bzrlib.plugin.load_from_path(['.'])
476 finally:
477- stream.close()
478+ if 'bzrlib.plugins.%s' % name in sys.modules:
479+ del sys.modules['bzrlib.plugins.%s' % name]
480+ if getattr(bzrlib.plugins, name, None):
481+ delattr(bzrlib.plugins, name)
482+ return stream.getvalue()
483
484 def test_plugin_with_bad_api_version_reports(self):
485 # This plugin asks for bzrlib api version 1.0.0, which is not supported
486
487=== modified file 'bzrlib/trace.py'
488--- bzrlib/trace.py 2009-11-19 20:47:51 +0000
489+++ bzrlib/trace.py 2009-11-27 06:20:25 +0000
490@@ -1,4 +1,4 @@
491-# Copyright (C) 2005, 2006, 2007, 2008 Canonical Ltd
492+# Copyright (C) 2005, 2006, 2007, 2008, 2009 Canonical Ltd
493 #
494 # This program is free software; you can redistribute it and/or modify
495 # it under the terms of the GNU General Public License as published by
496@@ -16,16 +16,17 @@
497
498 """Messages and logging for bazaar-ng.
499
500+The current general approach is that information for the user in normal
501+operation should go through the UIFactory, and trace should mostly be used for
502+debug and internal error information. However, this is not fully implemented.
503+
504 Messages are supplied by callers as a string-formatting template, plus values
505 to be inserted into it. The actual %-formatting is deferred to the log
506 library so that it doesn't need to be done for messages that won't be emitted.
507
508-Messages are classified by severity levels: critical, error, warning, info,
509-and debug.
510-
511-They can be sent to two places: to stderr, and to ~/.bzr.log. For purposes
512-such as running the test suite, they can also be redirected away from both of
513-those two places to another location.
514+They can be sent to two places: to stderr (via the UIFactory), and to
515+~/.bzr.log. For purposes such as running the test suite, they can also be
516+redirected away from both of those two places to another location.
517
518 ~/.bzr.log gets all messages, and full tracebacks for uncaught exceptions.
519 This trace file is always in UTF-8, regardless of the user's default encoding,
520@@ -46,17 +47,7 @@
521 form.
522 """
523
524-# FIXME: Unfortunately it turns out that python's logging module
525-# is quite expensive, even when the message is not printed by any handlers.
526-# We should perhaps change back to just simply doing it here.
527-#
528-# On the other hand, as of 1.2 we generally only call the mutter() statement
529-# if (according to debug_flags) we actually intend to write it. So the
530-# increased cost of logging.py is not so bad, and we could standardize on
531-# that.
532-
533 import codecs
534-import logging
535 import os
536 import sys
537 import time
538@@ -83,6 +74,8 @@
539 osutils,
540 plugin,
541 symbol_versioning,
542+ trace,
543+ ui,
544 )
545 """)
546
547@@ -106,43 +99,47 @@
548 _bzr_log_start_time = bzrlib._start_time
549
550
551-# held in a global for quick reference
552-_bzr_logger = logging.getLogger('bzr')
553-
554+def _fmt_msg(args, kwargs):
555+ # common compatibility code for functions that take dual format string
556+ # arguments; normally we don't want to do this from now on
557+ if len(args) > 1:
558+ if kwargs:
559+ raise ValueError("can't use both args and kwargs: %r, %r"
560+ % (args, kwargs))
561+ msg = args[0]
562+ return msg % args[1:]
563+ elif kwargs:
564+ return args[0] % kwargs
565+ else:
566+ # for compatibility we also accept passing objects that are then cast
567+ # down
568+ return unicode(args[0])
569+
570
571 def note(*args, **kwargs):
572- # FIXME note always emits utf-8, regardless of the terminal encoding
573- #
574- # FIXME: clearing the ui and then going through the abstract logging
575- # framework is whack; we should probably have a logging Handler that
576- # deals with terminal output if needed.
577- import bzrlib.ui
578- bzrlib.ui.ui_factory.clear_term()
579- _bzr_logger.info(*args, **kwargs)
580+ ui.ui_factory.show_message(_fmt_msg(args, kwargs))
581
582
583 def warning(*args, **kwargs):
584- import bzrlib.ui
585- bzrlib.ui.ui_factory.clear_term()
586- _bzr_logger.warning(*args, **kwargs)
587+ ui.ui_factory.show_warning(_fmt_msg(args, kwargs))
588
589
590 @deprecated_function(deprecated_in((2, 1, 0)))
591 def info(*args, **kwargs):
592 """Deprecated: use trace.note instead."""
593- note(*args, **kwargs)
594+ ui.ui_factory.show_message(_fmt_msg(args, kwargs))
595
596
597 @deprecated_function(deprecated_in((2, 1, 0)))
598 def log_error(*args, **kwargs):
599 """Deprecated: use bzrlib.trace.show_error instead"""
600- _bzr_logger.error(*args, **kwargs)
601+ ui.ui_factory.show_error(_fmt_msg(args, kwargs))
602
603
604 @deprecated_function(deprecated_in((2, 1, 0)))
605 def error(*args, **kwargs):
606 """Deprecated: use bzrlib.trace.show_error instead"""
607- _bzr_logger.error(*args, **kwargs)
608+ ui.ui_factory.show_error(_fmt_msg(args, kwargs))
609
610
611 def show_error(msg):
612@@ -150,7 +147,7 @@
613
614 Don't use this for exceptions, use report_exception instead.
615 """
616- _bzr_logger.error(*args, **kwargs)
617+ ui.ui_factory.show_error(msg)
618
619
620 _last_mutter_flush_time = None
621@@ -273,10 +270,7 @@
622 This should only be called once per process.
623
624 Non-command-line programs embedding bzrlib do not need to call this. They
625- can instead either pass a file to _push_log_file, or act directly on
626- logging.getLogger("bzr").
627-
628- Output can be redirected away by calling _push_log_file.
629+ can instead pass a file to _push_log_file.
630 """
631 # Do this before we open the log file, so we prevent
632 # get_terminal_encoding() from mutter()ing multiple times
633@@ -290,13 +284,6 @@
634 push_log_file(bzr_log_file,
635 r'[%(process)5d] %(asctime)s.%(msecs)03d %(levelname)s: %(message)s',
636 r'%Y-%m-%d %H:%M:%S')
637- # after hooking output into bzr_log, we also need to attach a stderr
638- # handler, writing only at level info and with encoding
639- writer_factory = codecs.getwriter(term_encoding)
640- encoded_stderr = writer_factory(sys.stderr, errors='replace')
641- stderr_handler = logging.StreamHandler(encoded_stderr)
642- stderr_handler.setLevel(logging.INFO)
643- logging.getLogger('bzr').addHandler(stderr_handler)
644
645
646 def push_log_file(to_file, log_format=None, date_format=None):
647@@ -305,37 +292,18 @@
648 :param to_file: A file-like object to which messages will be sent.
649
650 :returns: A memento that should be passed to _pop_log_file to restore the
651- previously active logging.
652+ previously active logging.
653 """
654 global _trace_file
655- # make a new handler
656- new_handler = logging.StreamHandler(to_file)
657- new_handler.setLevel(logging.DEBUG)
658- if log_format is None:
659- log_format = '%(levelname)8s %(message)s'
660- new_handler.setFormatter(logging.Formatter(log_format, date_format))
661- # save and remove any existing log handlers
662- bzr_logger = logging.getLogger('bzr')
663- old_handlers = bzr_logger.handlers[:]
664- del bzr_logger.handlers[:]
665- # set that as the default logger
666- bzr_logger.addHandler(new_handler)
667- bzr_logger.setLevel(logging.DEBUG)
668- # TODO: check if any changes are needed to the root logger
669- #
670- # TODO: also probably need to save and restore the level on bzr_logger.
671- # but maybe we can avoid setting the logger level altogether, and just set
672- # the level on the handler?
673- #
674 # save the old trace file
675 old_trace_file = _trace_file
676 # send traces to the new one
677 _trace_file = to_file
678- result = new_handler, _trace_file
679- return ('log_memento', old_handlers, new_handler, old_trace_file, to_file)
680-
681-
682-def pop_log_file((magic, old_handlers, new_handler, old_trace_file, new_trace_file)):
683+ result = _trace_file
684+ return ('log_memento', old_trace_file, to_file)
685+
686+
687+def pop_log_file((magic, old_trace_file, new_trace_file)):
688 """Undo changes to logging/tracing done by _push_log_file.
689
690 This flushes, but does not close the trace file.
691@@ -343,12 +311,6 @@
692 Takes the memento returned from _push_log_file."""
693 global _trace_file
694 _trace_file = old_trace_file
695- bzr_logger = logging.getLogger('bzr')
696- bzr_logger.removeHandler(new_handler)
697- # must be closed, otherwise logging will try to close it atexit, and the
698- # file will likely already be closed underneath.
699- new_handler.close()
700- bzr_logger.handlers = old_handlers
701 new_trace_file.flush()
702
703
704@@ -369,7 +331,6 @@
705 """
706 global _verbosity_level
707 _verbosity_level = level
708- _update_logging_level(level < 0)
709
710
711 def get_verbosity_level():
712@@ -388,14 +349,6 @@
713 set_verbosity_level(0)
714
715
716-def _update_logging_level(quiet=True):
717- """Hide INFO messages if quiet."""
718- if quiet:
719- _bzr_logger.setLevel(logging.WARNING)
720- else:
721- _bzr_logger.setLevel(logging.INFO)
722-
723-
724 def is_quiet():
725 """Is the verbosity level negative?"""
726 return _verbosity_level < 0
727
728=== modified file 'bzrlib/transport/ssh.py'
729--- bzrlib/transport/ssh.py 2009-11-25 07:27:43 +0000
730+++ bzrlib/transport/ssh.py 2009-11-27 06:20:25 +0000
731@@ -19,7 +19,6 @@
732
733 import errno
734 import getpass
735-import logging
736 import os
737 import socket
738 import subprocess
739@@ -500,6 +499,7 @@
740 # auth_none check to see if it is even supported.
741 supported_auth_types = []
742 try:
743+ import logging
744 # Note that with paramiko <1.7.5 this logs an INFO message:
745 # Authentication type (none) not permitted.
746 # So we explicitly disable the logging level for this action
747
748=== modified file 'bzrlib/ui/__init__.py'
749--- bzrlib/ui/__init__.py 2009-10-15 20:04:37 +0000
750+++ bzrlib/ui/__init__.py 2009-11-27 06:20:25 +0000
751@@ -39,6 +39,10 @@
752 depending on the detected capabilities of the terminal.
753 GUIs may choose to subclass this so that unimplemented methods fall
754 back to working through the terminal.
755+
756+bzrlib.tests.TestUIFactory
757+ Similar to TextUIFactory, but with no progress bars and some facilities
758+ for automated testing.
759 """
760
761
762
763=== modified file 'bzrlib/ui/text.py'
764--- bzrlib/ui/text.py 2009-09-23 06:29:46 +0000
765+++ bzrlib/ui/text.py 2009-11-27 06:20:25 +0000
766@@ -18,6 +18,7 @@
767 """Text UI, write output to the console.
768 """
769
770+import codecs
771 import getpass
772 import os
773 import sys
774@@ -44,18 +45,18 @@
775 class TextUIFactory(UIFactory):
776 """A UI factory for Text user interefaces."""
777
778- def __init__(self,
779- stdin=None,
780- stdout=None,
781- stderr=None):
782+ def __init__(self, stdin, stdout, stderr):
783 """Create a TextUIFactory.
784 """
785 super(TextUIFactory, self).__init__()
786- # TODO: there's no good reason not to pass all three streams, maybe we
787- # should deprecate the default values...
788 self.stdin = stdin
789 self.stdout = stdout
790 self.stderr = stderr
791+ # encoding_stderr is used for data put into the terminal encoding with
792+ # unrepresentable messages replaced; it should be used for non-bulk
793+ # messages
794+ self.message_stream = codecs.getwriter(osutils.get_terminal_encoding())(
795+ self.stderr, errors='replace')
796 # paints progress, network activity, etc
797 self._progress_view = self.make_progress_view()
798
799@@ -149,7 +150,9 @@
800 def note(self, msg):
801 """Write an already-formatted message, clearing the progress bar if necessary."""
802 self.clear_term()
803- self.stdout.write(msg + '\n')
804+ self.message_stream.write(unicode(msg) + '\n')
805+ # XXX: At the moment this accepts string-able objects, but we should
806+ # deprecate that and make the caller pass unicode.
807
808 def prompt(self, prompt, **kwargs):
809 """Emit prompt on the CLI.
810@@ -162,7 +165,7 @@
811 prompt = prompt % kwargs
812 prompt = prompt.encode(osutils.get_terminal_encoding(), 'replace')
813 self.clear_term()
814- self.stderr.write(prompt)
815+ self.message_stream.write(prompt)
816
817 def report_transport_activity(self, transport, byte_count, direction):
818 """Called by transports as they do IO.
819@@ -175,14 +178,15 @@
820
821 def show_error(self, msg):
822 self.clear_term()
823- self.stderr.write("bzr: error: %s\n" % msg)
824+ self.message_stream.write("bzr: error: %s\n" % msg)
825
826 def show_message(self, msg):
827- self.note(msg)
828+ self.clear_term()
829+ self.message_stream.write(unicode(msg) + '\n')
830
831 def show_warning(self, msg):
832 self.clear_term()
833- self.stderr.write("bzr: warning: %s\n" % msg)
834+ self.message_stream.write("bzr: warning: %s\n" % msg)
835
836 def _progress_updated(self, task):
837 """A task has been updated and wants to be displayed.
838
839=== modified file 'doc/developers/index.txt'
840--- doc/developers/index.txt 2009-11-18 02:09:01 +0000
841+++ doc/developers/index.txt 2009-11-27 06:20:25 +0000
842@@ -33,6 +33,7 @@
843
844 overview
845 integration
846+ user-interaction
847
848 * `Writing plugins for Bazaar <http://bazaar-vcs.org/WritingPlugins>`_ (web link)
849
850
851=== added file 'doc/developers/user-interaction.txt'
852--- doc/developers/user-interaction.txt 1970-01-01 00:00:00 +0000
853+++ doc/developers/user-interaction.txt 2009-11-27 06:20:25 +0000
854@@ -0,0 +1,62 @@
855+************************************
856+Bazaar User Interaction Architecture
857+************************************
858+
859+Bazaar aims to provide a library that can be mated to various text and
860+graphical user interfaces, and that can be used when there is no user to
861+interact with. This document describes some patterns and standards to
862+abstract this. It's a work in progress and not all code follows this
863+standard yet.
864+
865+Most interaction with the user goes through a ``UIFactory`` subclass bound
866+to the global variable ``bzrlib.ui.ui_factory``. Through this you can
867+display messages and ask for input. Data between the UIFactory and the
868+rest of the library should generally be Unicode, with the UIFactory
869+responsible for any encoding.
870+
871+Some UIFactories support only a subset of operations: for example
872+non-interactive UIs may refuse to ask for input.
873+
874+Plugin UIFactories might choose to subclass ``TextUIFactory`` rather than
875+the base UIFactory. Then if they don't have an implementation for a
876+particular method, it'll be done through text interaction in their
877+terminal window (if any) rather than giving an error.
878+
879+In general we want to add more semantic methods to the UIFactory so that
880+non-text user interfaces can do more than just present the text that would
881+have been shown in the console. For instance, rather than just presenting
882+the string that a branch is locked, we'd prefer to have a specific method
883+advising of inability to lock, so that a GUI could offer different options
884+or a specific presentation.
885+
886+We're moving towards a complete API separation between developer-oriented
887+debug messages, recorded using ``mutter``, and user-oriented messages.
888+The former can still be recorded even in graphical or non-interactive
889+programs.
890+
891+Core library code has the option to synchronously show messages to the
892+user or to ask for input through eg ``get_boolean`` or ``show_message``,
893+but it should do this with care. In a GUI the user should be driving the
894+interaction, not the application.
895+
896+Some core library code, such as upgrade or checkout, does some work that
897+should be reported to the user in a text-mode application, but that might
898+be shown very differently or not at all in other cases. Rather than
899+repeatedly showing messages.
900+
901+A particular case of this is the ProgressTask api, where a model/view
902+separation is fairly complete. Core code obtains a ``ProgressTask`` from
903+``ui.ui_factory.nested_progress_bar``, and then feeds it events to tell
904+how the task is progressing, with no concern for whether or how this is
905+displayed to the user.
906+
907+In tests, mutter output is collected onto the test log, so that it can be
908+shown when there's a failure or inspected by code that wants to check what
909+was logged. However, this should no longer be used for inspecting user
910+output: do that by providing a custom UIFactory or using run_bzr_captured.
911+
912+stdout and stderr
913+*****************
914+
915+We normally reserve stdout for bulk output data, such as diffs, logs, or
916+bundles, and put all UIFactory output onto stderr.