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

Proposed by Martin Pool
Status: Superseded
Proposed branch: lp:~mbp/bzr/remove-logging
Merge into: lp:bzr
Diff against target: 903 lines (+196/-208)
21 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/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
John A Meinel Needs Information
Michael Hudson-Doyle Pending
Review via email: mp+14942@code.launchpad.net

This proposal has been superseded by a proposal from 2009-11-27.

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

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.

lp:~mbp/bzr/remove-logging updated
4804. By Canonical.com Patch Queue Manager <email address hidden>

(mbp, trivial) further consistency of sphinx in Japanese

4805. By Canonical.com Patch Queue Manager <email address hidden>

(mbp) really fix Japanese sphinx

Revision history for this message
John A Meinel (jameinel) wrote :
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 :

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

lp:~mbp/bzr/remove-logging updated
4806. By Canonical.com Patch Queue Manager <email address hidden>

(jam, Martin gz) More win32 fixes.

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

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

lp:~mbp/bzr/remove-logging updated
4807. By Canonical.com Patch Queue Manager <email address hidden>

(mbp, trivial) additional doc building fix

4808. By Canonical.com Patch Queue Manager <email address hidden>

(vila) pycurl can't always connect direct to 'localhost' because of
 IPv4/v6 changes.

4809. By Canonical.com Patch Queue Manager <email address hidden>

(Martin <gzlist>) Allow specifying path in 'BZR_SSH' rather than just
 vendors.

4810. By Canonical.com Patch Queue Manager <email address hidden>

(jam) Last few tweaks to get the win32 test suite to pass.

4811. By Canonical.com Patch Queue Manager <email address hidden>

(Andrew Bennetts) Add 'Bazaar Contribution in Five Minutes'
 introduction to developer docs.

4812. By Canonical.com Patch Queue Manager <email address hidden>

(Alexander Sack) Add --commit-time option to 'bzr commit'. (#459276)

4813. By Canonical.com Patch Queue Manager <email address hidden>

(jam) Remove a @needs_read_lock decorator from something that doesn't
 really need it.

4814. By Canonical.com Patch Queue Manager <email address hidden>

(jam) Release the gil during some of the core groupcompress code
 paths.

4815. By Canonical.com Patch Queue Manager <email address hidden>

(jam) Fix CommitBuilder.inv_sha1 when using record_iter_changes.

4816. By Canonical.com Patch Queue Manager <email address hidden>

(igc) Explain that .bzrignore is implicitly added (Patrick Regan,
 #59608)

4817. By Canonical.com Patch Queue Manager <email address hidden>

(igc) Trivial formatting fix to merge help

4818. By Canonical.com Patch Queue Manager <email address hidden>

(jam) Add a developer doc describing dependencies for win32 builds

4819. By Canonical.com Patch Queue Manager <email address hidden>

(jam) Fix bug #485771,
 only change slashes on arguments that are being globbed.

4820. By Canonical.com Patch Queue Manager <email address hidden>

(igc) better explanation of diff -c (#475451)

4821. By Canonical.com Patch Queue Manager <email address hidden>

(vila) Further clarifications on building releases

4822. By Canonical.com Patch Queue Manager <email address hidden>

* TestCaseWithMemoryTransport no longer sets $HOME and $BZR_HOME to
 unicode strings. (Michael Hudson, #464174)

4823. By Canonical.com Patch Queue Manager <email address hidden>

(robertc) Fix parallel selftest with subprocesses - needed for win32.
 (Gordon Tyler)

4824. By Canonical.com Patch Queue Manager <email address hidden>

(robertc) Be more clear in the help for bzr serve about the
 --allow-writes option. (Robert Collins)

4825. By Canonical.com Patch Queue Manager <email address hidden>

(andrew) Terminate ssh subprocesses when no references to them
 remain. (#426662)

4826. By Canonical.com Patch Queue Manager <email address hidden>

(robertc) Documentation fixes. (Neil Martinsen-Burrell, bugs 249919,
 358821, 486892).

4827. By Canonical.com Patch Queue Manager <email address hidden>

(robertc) Fix handling of -Derror for ValueError. (Benoit PIERRE)

4828. By Canonical.com Patch Queue Manager <email address hidden>

(mbp) remove bzrlib.textui

4829. By Canonical.com Patch Queue Manager <email address hidden>

(robertc) Doc improvements for bzr help urlspec. (Neil
 Martinsen-Burrell)

4830. By Canonical.com Patch Queue Manager <email address hidden>

bzrlib.urlutils.local_path_from_url now accepts file://localhost/ as
 well as file:/// URLs on POSIX. (Michael Hudson)

4831. By Canonical.com Patch Queue Manager <email address hidden>

(bialix) Shelve command refuse to run if there is no real terminal

4832. By Martin Pool

Add user interaction architecture doc

4833. By Martin Pool

Tags can be tested in the default format now

4834. By Martin Pool

Document bzrlib.tests.TestUIFactory

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.

4836. By Martin Pool

Cope when sys.exitfunc doesn't exist

4837. By Martin Pool

Remove redundant 'warning' prefix from some messages

4838. By Martin Pool

Correction to error handling test that depended on python logging behaviour

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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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 05:14:10 +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/test_plugins.py'
408--- bzrlib/tests/test_plugins.py 2009-09-18 14:53:37 +0000
409+++ bzrlib/tests/test_plugins.py 2009-11-27 05:14:10 +0000
410@@ -1,4 +1,4 @@
411-# Copyright (C) 2005, 2007 Canonical Ltd
412+# Copyright (C) 2005, 2007, 2009 Canonical Ltd
413 #
414 # This program is free software; you can redistribute it and/or modify
415 # it under the terms of the GNU General Public License as published by
416@@ -16,11 +16,6 @@
417
418 """Tests for plugins"""
419
420-# XXX: There are no plugin tests at the moment because the plugin module
421-# affects the global state of the process. See bzrlib/plugins.py for more
422-# comments.
423-
424-import logging
425 import os
426 from StringIO import StringIO
427 import sys
428@@ -30,6 +25,7 @@
429 osutils,
430 plugin,
431 tests,
432+ ui,
433 )
434 import bzrlib.plugin
435 import bzrlib.plugins
436@@ -202,28 +198,17 @@
437 :param name: The name of the plugin.
438 :return: A string with the log from the plugin loading call.
439 """
440- # Capture output
441 stream = StringIO()
442+ ui.ui_factory = tests.TestUIFactory(stdout=stream, stderr=stream,
443+ stdin=None)
444 try:
445- handler = logging.StreamHandler(stream)
446- log = logging.getLogger('bzr')
447- log.addHandler(handler)
448- try:
449- try:
450- bzrlib.plugin.load_from_path(['.'])
451- finally:
452- if 'bzrlib.plugins.%s' % name in sys.modules:
453- del sys.modules['bzrlib.plugins.%s' % name]
454- if getattr(bzrlib.plugins, name, None):
455- delattr(bzrlib.plugins, name)
456- finally:
457- # Stop capturing output
458- handler.flush()
459- handler.close()
460- log.removeHandler(handler)
461- return stream.getvalue()
462+ bzrlib.plugin.load_from_path(['.'])
463 finally:
464- stream.close()
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+ return stream.getvalue()
470
471 def test_plugin_with_bad_api_version_reports(self):
472 # This plugin asks for bzrlib api version 1.0.0, which is not supported
473
474=== modified file 'bzrlib/trace.py'
475--- bzrlib/trace.py 2009-11-19 20:47:51 +0000
476+++ bzrlib/trace.py 2009-11-27 05:14:10 +0000
477@@ -1,4 +1,4 @@
478-# Copyright (C) 2005, 2006, 2007, 2008 Canonical Ltd
479+# Copyright (C) 2005, 2006, 2007, 2008, 2009 Canonical Ltd
480 #
481 # This program is free software; you can redistribute it and/or modify
482 # it under the terms of the GNU General Public License as published by
483@@ -16,16 +16,17 @@
484
485 """Messages and logging for bazaar-ng.
486
487+The current general approach is that information for the user in normal
488+operation should go through the UIFactory, and trace should mostly be used for
489+debug and internal error information. However, this is not fully implemented.
490+
491 Messages are supplied by callers as a string-formatting template, plus values
492 to be inserted into it. The actual %-formatting is deferred to the log
493 library so that it doesn't need to be done for messages that won't be emitted.
494
495-Messages are classified by severity levels: critical, error, warning, info,
496-and debug.
497-
498-They can be sent to two places: to stderr, and to ~/.bzr.log. For purposes
499-such as running the test suite, they can also be redirected away from both of
500-those two places to another location.
501+They can be sent to two places: to stderr (via the UIFactory), and to
502+~/.bzr.log. For purposes such as running the test suite, they can also be
503+redirected away from both of those two places to another location.
504
505 ~/.bzr.log gets all messages, and full tracebacks for uncaught exceptions.
506 This trace file is always in UTF-8, regardless of the user's default encoding,
507@@ -46,17 +47,7 @@
508 form.
509 """
510
511-# FIXME: Unfortunately it turns out that python's logging module
512-# is quite expensive, even when the message is not printed by any handlers.
513-# We should perhaps change back to just simply doing it here.
514-#
515-# On the other hand, as of 1.2 we generally only call the mutter() statement
516-# if (according to debug_flags) we actually intend to write it. So the
517-# increased cost of logging.py is not so bad, and we could standardize on
518-# that.
519-
520 import codecs
521-import logging
522 import os
523 import sys
524 import time
525@@ -83,6 +74,8 @@
526 osutils,
527 plugin,
528 symbol_versioning,
529+ trace,
530+ ui,
531 )
532 """)
533
534@@ -106,43 +99,47 @@
535 _bzr_log_start_time = bzrlib._start_time
536
537
538-# held in a global for quick reference
539-_bzr_logger = logging.getLogger('bzr')
540-
541+def _fmt_msg(args, kwargs):
542+ # common compatibility code for functions that take dual format string
543+ # arguments; normally we don't want to do this from now on
544+ if len(args) > 1:
545+ if kwargs:
546+ raise ValueError("can't use both args and kwargs: %r, %r"
547+ % (args, kwargs))
548+ msg = args[0]
549+ return msg % args[1:]
550+ elif kwargs:
551+ return args[0] % kwargs
552+ else:
553+ # for compatibility we also accept passing objects that are then cast
554+ # down
555+ return unicode(args[0])
556+
557
558 def note(*args, **kwargs):
559- # FIXME note always emits utf-8, regardless of the terminal encoding
560- #
561- # FIXME: clearing the ui and then going through the abstract logging
562- # framework is whack; we should probably have a logging Handler that
563- # deals with terminal output if needed.
564- import bzrlib.ui
565- bzrlib.ui.ui_factory.clear_term()
566- _bzr_logger.info(*args, **kwargs)
567+ ui.ui_factory.show_message(_fmt_msg(args, kwargs))
568
569
570 def warning(*args, **kwargs):
571- import bzrlib.ui
572- bzrlib.ui.ui_factory.clear_term()
573- _bzr_logger.warning(*args, **kwargs)
574+ ui.ui_factory.show_warning(_fmt_msg(args, kwargs))
575
576
577 @deprecated_function(deprecated_in((2, 1, 0)))
578 def info(*args, **kwargs):
579 """Deprecated: use trace.note instead."""
580- note(*args, **kwargs)
581+ ui.ui_factory.show_message(_fmt_msg(args, kwargs))
582
583
584 @deprecated_function(deprecated_in((2, 1, 0)))
585 def log_error(*args, **kwargs):
586 """Deprecated: use bzrlib.trace.show_error instead"""
587- _bzr_logger.error(*args, **kwargs)
588+ ui.ui_factory.show_error(_fmt_msg(args, kwargs))
589
590
591 @deprecated_function(deprecated_in((2, 1, 0)))
592 def error(*args, **kwargs):
593 """Deprecated: use bzrlib.trace.show_error instead"""
594- _bzr_logger.error(*args, **kwargs)
595+ ui.ui_factory.show_error(_fmt_msg(args, kwargs))
596
597
598 def show_error(msg):
599@@ -150,7 +147,7 @@
600
601 Don't use this for exceptions, use report_exception instead.
602 """
603- _bzr_logger.error(*args, **kwargs)
604+ ui.ui_factory.show_error(msg)
605
606
607 _last_mutter_flush_time = None
608@@ -273,10 +270,7 @@
609 This should only be called once per process.
610
611 Non-command-line programs embedding bzrlib do not need to call this. They
612- can instead either pass a file to _push_log_file, or act directly on
613- logging.getLogger("bzr").
614-
615- Output can be redirected away by calling _push_log_file.
616+ can instead pass a file to _push_log_file.
617 """
618 # Do this before we open the log file, so we prevent
619 # get_terminal_encoding() from mutter()ing multiple times
620@@ -290,13 +284,6 @@
621 push_log_file(bzr_log_file,
622 r'[%(process)5d] %(asctime)s.%(msecs)03d %(levelname)s: %(message)s',
623 r'%Y-%m-%d %H:%M:%S')
624- # after hooking output into bzr_log, we also need to attach a stderr
625- # handler, writing only at level info and with encoding
626- writer_factory = codecs.getwriter(term_encoding)
627- encoded_stderr = writer_factory(sys.stderr, errors='replace')
628- stderr_handler = logging.StreamHandler(encoded_stderr)
629- stderr_handler.setLevel(logging.INFO)
630- logging.getLogger('bzr').addHandler(stderr_handler)
631
632
633 def push_log_file(to_file, log_format=None, date_format=None):
634@@ -305,37 +292,18 @@
635 :param to_file: A file-like object to which messages will be sent.
636
637 :returns: A memento that should be passed to _pop_log_file to restore the
638- previously active logging.
639+ previously active logging.
640 """
641 global _trace_file
642- # make a new handler
643- new_handler = logging.StreamHandler(to_file)
644- new_handler.setLevel(logging.DEBUG)
645- if log_format is None:
646- log_format = '%(levelname)8s %(message)s'
647- new_handler.setFormatter(logging.Formatter(log_format, date_format))
648- # save and remove any existing log handlers
649- bzr_logger = logging.getLogger('bzr')
650- old_handlers = bzr_logger.handlers[:]
651- del bzr_logger.handlers[:]
652- # set that as the default logger
653- bzr_logger.addHandler(new_handler)
654- bzr_logger.setLevel(logging.DEBUG)
655- # TODO: check if any changes are needed to the root logger
656- #
657- # TODO: also probably need to save and restore the level on bzr_logger.
658- # but maybe we can avoid setting the logger level altogether, and just set
659- # the level on the handler?
660- #
661 # save the old trace file
662 old_trace_file = _trace_file
663 # send traces to the new one
664 _trace_file = to_file
665- result = new_handler, _trace_file
666- return ('log_memento', old_handlers, new_handler, old_trace_file, to_file)
667-
668-
669-def pop_log_file((magic, old_handlers, new_handler, old_trace_file, new_trace_file)):
670+ result = _trace_file
671+ return ('log_memento', old_trace_file, to_file)
672+
673+
674+def pop_log_file((magic, old_trace_file, new_trace_file)):
675 """Undo changes to logging/tracing done by _push_log_file.
676
677 This flushes, but does not close the trace file.
678@@ -343,12 +311,6 @@
679 Takes the memento returned from _push_log_file."""
680 global _trace_file
681 _trace_file = old_trace_file
682- bzr_logger = logging.getLogger('bzr')
683- bzr_logger.removeHandler(new_handler)
684- # must be closed, otherwise logging will try to close it atexit, and the
685- # file will likely already be closed underneath.
686- new_handler.close()
687- bzr_logger.handlers = old_handlers
688 new_trace_file.flush()
689
690
691@@ -369,7 +331,6 @@
692 """
693 global _verbosity_level
694 _verbosity_level = level
695- _update_logging_level(level < 0)
696
697
698 def get_verbosity_level():
699@@ -388,14 +349,6 @@
700 set_verbosity_level(0)
701
702
703-def _update_logging_level(quiet=True):
704- """Hide INFO messages if quiet."""
705- if quiet:
706- _bzr_logger.setLevel(logging.WARNING)
707- else:
708- _bzr_logger.setLevel(logging.INFO)
709-
710-
711 def is_quiet():
712 """Is the verbosity level negative?"""
713 return _verbosity_level < 0
714
715=== modified file 'bzrlib/transport/ssh.py'
716--- bzrlib/transport/ssh.py 2009-11-25 07:27:43 +0000
717+++ bzrlib/transport/ssh.py 2009-11-27 05:14:10 +0000
718@@ -19,7 +19,6 @@
719
720 import errno
721 import getpass
722-import logging
723 import os
724 import socket
725 import subprocess
726@@ -500,6 +499,7 @@
727 # auth_none check to see if it is even supported.
728 supported_auth_types = []
729 try:
730+ import logging
731 # Note that with paramiko <1.7.5 this logs an INFO message:
732 # Authentication type (none) not permitted.
733 # So we explicitly disable the logging level for this action
734
735=== modified file 'bzrlib/ui/__init__.py'
736--- bzrlib/ui/__init__.py 2009-10-15 20:04:37 +0000
737+++ bzrlib/ui/__init__.py 2009-11-27 05:14:10 +0000
738@@ -39,6 +39,10 @@
739 depending on the detected capabilities of the terminal.
740 GUIs may choose to subclass this so that unimplemented methods fall
741 back to working through the terminal.
742+
743+bzrlib.tests.TestUIFactory
744+ Similar to TextUIFactory, but with no progress bars and some facilities
745+ for automated testing.
746 """
747
748
749
750=== modified file 'bzrlib/ui/text.py'
751--- bzrlib/ui/text.py 2009-09-23 06:29:46 +0000
752+++ bzrlib/ui/text.py 2009-11-27 05:14:10 +0000
753@@ -18,6 +18,7 @@
754 """Text UI, write output to the console.
755 """
756
757+import codecs
758 import getpass
759 import os
760 import sys
761@@ -44,18 +45,18 @@
762 class TextUIFactory(UIFactory):
763 """A UI factory for Text user interefaces."""
764
765- def __init__(self,
766- stdin=None,
767- stdout=None,
768- stderr=None):
769+ def __init__(self, stdin, stdout, stderr):
770 """Create a TextUIFactory.
771 """
772 super(TextUIFactory, self).__init__()
773- # TODO: there's no good reason not to pass all three streams, maybe we
774- # should deprecate the default values...
775 self.stdin = stdin
776 self.stdout = stdout
777 self.stderr = stderr
778+ # encoding_stderr is used for data put into the terminal encoding with
779+ # unrepresentable messages replaced; it should be used for non-bulk
780+ # messages
781+ self.message_stream = codecs.getwriter(osutils.get_terminal_encoding())(
782+ self.stderr, errors='replace')
783 # paints progress, network activity, etc
784 self._progress_view = self.make_progress_view()
785
786@@ -149,7 +150,9 @@
787 def note(self, msg):
788 """Write an already-formatted message, clearing the progress bar if necessary."""
789 self.clear_term()
790- self.stdout.write(msg + '\n')
791+ self.message_stream.write(unicode(msg) + '\n')
792+ # XXX: At the moment this accepts string-able objects, but we should
793+ # deprecate that and make the caller pass unicode.
794
795 def prompt(self, prompt, **kwargs):
796 """Emit prompt on the CLI.
797@@ -162,7 +165,7 @@
798 prompt = prompt % kwargs
799 prompt = prompt.encode(osutils.get_terminal_encoding(), 'replace')
800 self.clear_term()
801- self.stderr.write(prompt)
802+ self.message_stream.write(prompt)
803
804 def report_transport_activity(self, transport, byte_count, direction):
805 """Called by transports as they do IO.
806@@ -175,14 +178,15 @@
807
808 def show_error(self, msg):
809 self.clear_term()
810- self.stderr.write("bzr: error: %s\n" % msg)
811+ self.message_stream.write("bzr: error: %s\n" % msg)
812
813 def show_message(self, msg):
814- self.note(msg)
815+ self.clear_term()
816+ self.message_stream.write(unicode(msg) + '\n')
817
818 def show_warning(self, msg):
819 self.clear_term()
820- self.stderr.write("bzr: warning: %s\n" % msg)
821+ self.message_stream.write("bzr: warning: %s\n" % msg)
822
823 def _progress_updated(self, task):
824 """A task has been updated and wants to be displayed.
825
826=== modified file 'doc/developers/index.txt'
827--- doc/developers/index.txt 2009-11-18 02:09:01 +0000
828+++ doc/developers/index.txt 2009-11-27 05:14:10 +0000
829@@ -33,6 +33,7 @@
830
831 overview
832 integration
833+ user-interaction
834
835 * `Writing plugins for Bazaar <http://bazaar-vcs.org/WritingPlugins>`_ (web link)
836
837
838=== added file 'doc/developers/user-interaction.txt'
839--- doc/developers/user-interaction.txt 1970-01-01 00:00:00 +0000
840+++ doc/developers/user-interaction.txt 2009-11-27 05:14:10 +0000
841@@ -0,0 +1,62 @@
842+************************************
843+Bazaar User Interaction Architecture
844+************************************
845+
846+Bazaar aims to provide a library that can be mated to various text and
847+graphical user interfaces, and that can be used when there is no user to
848+interact with. This document describes some patterns and standards to
849+abstract this. It's a work in progress and not all code follows this
850+standard yet.
851+
852+Most interaction with the user goes through a ``UIFactory`` subclass bound
853+to the global variable ``bzrlib.ui.ui_factory``. Through this you can
854+display messages and ask for input. Data between the UIFactory and the
855+rest of the library should generally be Unicode, with the UIFactory
856+responsible for any encoding.
857+
858+Some UIFactories support only a subset of operations: for example
859+non-interactive UIs may refuse to ask for input.
860+
861+Plugin UIFactories might choose to subclass ``TextUIFactory`` rather than
862+the base UIFactory. Then if they don't have an implementation for a
863+particular method, it'll be done through text interaction in their
864+terminal window (if any) rather than giving an error.
865+
866+In general we want to add more semantic methods to the UIFactory so that
867+non-text user interfaces can do more than just present the text that would
868+have been shown in the console. For instance, rather than just presenting
869+the string that a branch is locked, we'd prefer to have a specific method
870+advising of inability to lock, so that a GUI could offer different options
871+or a specific presentation.
872+
873+We're moving towards a complete API separation between developer-oriented
874+debug messages, recorded using ``mutter``, and user-oriented messages.
875+The former can still be recorded even in graphical or non-interactive
876+programs.
877+
878+Core library code has the option to synchronously show messages to the
879+user or to ask for input through eg ``get_boolean`` or ``show_message``,
880+but it should do this with care. In a GUI the user should be driving the
881+interaction, not the application.
882+
883+Some core library code, such as upgrade or checkout, does some work that
884+should be reported to the user in a text-mode application, but that might
885+be shown very differently or not at all in other cases. Rather than
886+repeatedly showing messages.
887+
888+A particular case of this is the ProgressTask api, where a model/view
889+separation is fairly complete. Core code obtains a ``ProgressTask`` from
890+``ui.ui_factory.nested_progress_bar``, and then feeds it events to tell
891+how the task is progressing, with no concern for whether or how this is
892+displayed to the user.
893+
894+In tests, mutter output is collected onto the test log, so that it can be
895+shown when there's a failure or inspected by code that wants to check what
896+was logged. However, this should no longer be used for inspecting user
897+output: do that by providing a custom UIFactory or using run_bzr_captured.
898+
899+stdout and stderr
900+*****************
901+
902+We normally reserve stdout for bulk output data, such as diffs, logs, or
903+bundles, and put all UIFactory output onto stderr.