Merge lp:~mbp/bzr/initialize into lp:bzr

Proposed by Martin Pool
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/initialize
Merge into: lp:bzr
Diff against target: 252 lines (+101/-44)
5 files modified
NEWS (+5/-0)
bzr (+3/-18)
bzrlib/__init__.py (+49/-0)
bzrlib/commands.py (+23/-25)
bzrlib/trace.py (+21/-1)
To merge this branch: bzr merge lp:~mbp/bzr/initialize
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+18967@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

This adds a bzrlib.initialize function for the benefit of other applications using bzrlib, so that they don't need to know every particular component that needs to be initialized.

This may also be an easier place to do initialization than in code that runs at import time.

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

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

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/initialize into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #507710 want bzrlib.initialize() to do all typical setup
> https://bugs.launchpad.net/bugs/507710
>
>
> This adds a bzrlib.initialize function for the benefit of other applications using bzrlib, so that they don't need to know every particular component that needs to be initialized.
>
> This may also be an easier place to do initialization than in code that runs at import time.
>

This
 install_bzr_command_hooks()

Is still something that wrappers need to run before the have access to
the command infrastructure. Otherwise, AFAICT this covers all the stuff
that I would normally try to set up manually. (I'm just trying to
remember the various things that stuff like loggerhead has run into
trying to call into bzrlib.)

 merge: approve

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

iEYEARECAAYFAktyQjsACgkQJdeBCYSNAAOZ+wCfWE++nWxOsztbLhCauR8ByCln
dgAAn0w/vR0BDc9HXOOiRDBOijHl41C6
=pFMi
-----END PGP SIGNATURE-----

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

bb:approve

I haven't read this extremely carefully, as I assume it's a largely mechanical
change, but I like it.

I also like that it leaves less code in the 'bzr' script, which presumably will
save a few microseconds at startup (as the 'bzr' file doesn't have a pyc cached
for it)... :)

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

> This
> install_bzr_command_hooks()
>
> Is still something that wrappers need to run before the have access to
> the command infrastructure. Otherwise, AFAICT this covers all the stuff
> that I would normally try to set up manually. (I'm just trying to
> remember the various things that stuff like loggerhead has run into
> trying to call into bzrlib.)

I was going to add that too, but
1- it's a bit hard :-) therefore I wanted to land the easy part first

I forget the details but there are some ordering dependencies that mean you have to do this a bit later than the rest of the initialization.

2- At least some applications don't actually care about running bzr commands

3- I'd like to later make commands lazy loaded and will revisit this there.

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

> I forget the details but there are some ordering dependencies that mean you
> have to do this a bit later than the rest of the initialization.

Oh, as the comment indicates, the problem is that some of the command stuff is torn down on every test run, therefore they have to be reestablished on every run_bzr. This is a bit gross but out of scope for this patch.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-02-10 22:24:03 +0000
3+++ NEWS 2010-02-11 01:17:13 +0000
4@@ -49,6 +49,11 @@
5 API Changes
6 ***********
7
8+* New ``bzrlib.initialize`` is recommended for programs using bzrlib to
9+ run when starting up; it sets up several things that previously needed
10+ to be done separately.
11+ (Martin Pool, #507710)
12+
13 * Remove unused ``CommandFailed`` exception.
14 (Martin Pool)
15
16
17=== modified file 'bzr'
18--- bzr 2010-01-21 22:23:29 +0000
19+++ bzr 2010-02-11 01:17:13 +0000
20@@ -1,6 +1,6 @@
21 #! /usr/bin/env python
22
23-# Copyright (C) 2005, 2006, 2007, 2008, 2009 Canonical Ltd
24+# Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Canonical Ltd
25 #
26 # This program is free software; you can redistribute it and/or modify
27 # it under the terms of the GNU General Public License as published by
28@@ -138,16 +138,12 @@
29
30
31 if __name__ == '__main__':
32- bzrlib.trace.enable_default_logging()
33+ bzrlib.initialize()
34 exit_val = bzrlib.commands.main()
35
36 if profiling:
37 profile_imports.log_stack_info(sys.stderr)
38
39- # run anything registered by atexit, because it won't be run in the normal
40- # way
41- sys.exitfunc()
42-
43 # By this point we really have completed everything we want to do, and
44 # there's no point doing any additional cleanup. Abruptly exiting here
45 # stops any background threads getting into trouble as code is unloaded,
46@@ -155,18 +151,7 @@
47 # are just about to be discarded anyhow. This does mean that atexit hooks
48 # won't run but we don't use them. Also file buffers won't be flushed,
49 # but our policy is to always close files from a finally block. -- mbp 20070215
50- try:
51- sys.stdout.flush()
52- sys.stderr.flush()
53- except IOError, e:
54- import errno
55- if e.errno in [errno.EINVAL, errno.EPIPE]:
56- pass
57- else:
58- raise
59- if bzrlib.trace._trace_file:
60- # this is also _bzr_log
61- bzrlib.trace._trace_file.flush()
62+ sys.exitfunc()
63 os._exit(exit_val)
64 else:
65 raise ImportError("The bzr script cannot be imported.")
66
67=== modified file 'bzrlib/__init__.py'
68--- bzrlib/__init__.py 2010-01-29 11:09:30 +0000
69+++ bzrlib/__init__.py 2010-02-11 01:17:13 +0000
70@@ -116,3 +116,52 @@
71 def test_suite():
72 import tests
73 return tests.test_suite()
74+
75+
76+def initialize(
77+ setup_ui=True,
78+ stdin=None, stdout=None, stderr=None):
79+ """Set up everything needed for normal use of bzrlib.
80+
81+ Most applications that embed bzrlib, including bzr itself, should call
82+ this function to initialize various subsystems.
83+
84+ More options may be added in future so callers should use named arguments.
85+
86+ :param setup_ui: If true (default) use a terminal UI; otherwise
87+ something else must be put into `bzrlib.ui.ui_factory`.
88+ :param stdin, stdout, stderr: If provided, use these for terminal IO;
89+ otherwise use the files in `sys`.
90+ """
91+ # TODO: mention this in a guide to embedding bzrlib
92+ #
93+ # NB: This function tweaks so much global state it's hard to test it in
94+ # isolation within the same interpreter. It's not reached on normal
95+ # in-process run_bzr calls. If it's broken, we expect that
96+ # TestRunBzrSubprocess may fail.
97+
98+ import atexit
99+ import bzrlib.trace
100+
101+ bzrlib.trace.enable_default_logging()
102+ atexit.register(bzrlib.trace._flush_stdout_stderr)
103+ atexit.register(bzrlib.trace._flush_trace)
104+
105+ import bzrlib.ui
106+ if stdin is None:
107+ stdin = sys.stdin
108+ if stdout is None:
109+ stdout = sys.stdout
110+ if stderr is None:
111+ stderr = sys.stderr
112+
113+ if setup_ui:
114+ bzrlib.ui.ui_factory = bzrlib.ui.make_ui_for_terminal(
115+ stdin, stdout, stderr)
116+
117+ if bzrlib.version_info[3] == 'final':
118+ from bzrlib.symbol_versioning import suppress_deprecation_warnings
119+ suppress_deprecation_warnings(override=True)
120+
121+ import bzrlib.osutils
122+ atexit.register(osutils.report_extension_load_failures)
123
124=== modified file 'bzrlib/commands.py'
125--- bzrlib/commands.py 2010-02-10 17:52:08 +0000
126+++ bzrlib/commands.py 2010-02-11 01:17:13 +0000
127@@ -59,7 +59,6 @@
128 deprecated_function,
129 deprecated_in,
130 deprecated_method,
131- suppress_deprecation_warnings,
132 )
133
134
135@@ -1078,25 +1077,12 @@
136 "bzr plugin-provider-db check")
137
138
139-def main(argv=None):
140- """Main entry point of command-line interface.
141-
142- :param argv: list of unicode command-line arguments similar to sys.argv.
143- argv[0] is script name usually, it will be ignored.
144- Don't pass here sys.argv because this list contains plain strings
145- and not unicode; pass None instead.
146-
147- :return: exit code of bzr command.
148- """
149- import bzrlib.ui
150- bzrlib.ui.ui_factory = bzrlib.ui.make_ui_for_terminal(
151- sys.stdin, sys.stdout, sys.stderr)
152-
153- # Is this a final release version? If so, we should suppress warnings
154- if bzrlib.version_info[3] == 'final':
155- suppress_deprecation_warnings(override=True)
156+
157+def _specified_or_unicode_argv(argv):
158+ # For internal or testing use, argv can be passed. Otherwise, get it from
159+ # the process arguments in a unicode-safe way.
160 if argv is None:
161- argv = osutils.get_unicode_argv()
162+ return osutils.get_unicode_argv()
163 else:
164 new_argv = []
165 try:
166@@ -1108,12 +1094,26 @@
167 new_argv.append(a.decode('ascii'))
168 except UnicodeDecodeError:
169 raise errors.BzrError("argv should be list of unicode strings.")
170- argv = new_argv
171+ return new_argv
172+
173+
174+def main(argv=None):
175+ """Main entry point of command-line interface.
176+
177+ Typically `bzrlib.initialize` should be called first.
178+
179+ :param argv: list of unicode command-line arguments similar to sys.argv.
180+ argv[0] is script name usually, it will be ignored.
181+ Don't pass here sys.argv because this list contains plain strings
182+ and not unicode; pass None instead.
183+
184+ :return: exit code of bzr command.
185+ """
186+ argv = _specified_or_unicode_argv(argv)
187 ret = run_bzr_catch_errors(argv)
188 bzrlib.ui.ui_factory.log_transport_activity(
189 display=('bytes' in debug.debug_flags))
190 trace.mutter("return code %d", ret)
191- osutils.report_extension_load_failures()
192 return ret
193
194
195@@ -1123,6 +1123,7 @@
196 This function assumed that that UI layer is setup, that symbol deprecations
197 are already applied, and that unicode decoding has already been performed on argv.
198 """
199+ # done here so that they're covered for every test run
200 install_bzr_command_hooks()
201 return exception_to_return_code(run_bzr, argv)
202
203@@ -1133,6 +1134,7 @@
204 This is used for the test suite, and might be useful for other programs
205 that want to wrap the commandline interface.
206 """
207+ # done here so that they're covered for every test run
208 install_bzr_command_hooks()
209 try:
210 return run_bzr(argv)
211@@ -1188,7 +1190,3 @@
212 yield provider
213
214 command_providers_registry = ProvidersRegistry()
215-
216-
217-if __name__ == '__main__':
218- sys.exit(main(sys.argv))
219
220=== modified file 'bzrlib/trace.py'
221--- bzrlib/trace.py 2010-01-15 03:29:33 +0000
222+++ bzrlib/trace.py 2010-02-11 01:17:13 +0000
223@@ -1,4 +1,4 @@
224-# Copyright (C) 2005, 2006, 2007, 2008, 2009 Canonical Ltd
225+# Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Canonical Ltd
226 #
227 # This program is free software; you can redistribute it and/or modify
228 # it under the terms of the GNU General Public License as published by
229@@ -501,3 +501,23 @@
230 """Report an exception that probably indicates a bug in bzr"""
231 from bzrlib.crash import report_bug
232 report_bug(exc_info, err_file)
233+
234+
235+def _flush_stdout_stderr():
236+ # installed into an atexit hook by bzrlib.initialize()
237+ try:
238+ sys.stdout.flush()
239+ sys.stderr.flush()
240+ except IOError, e:
241+ import errno
242+ if e.errno in [errno.EINVAL, errno.EPIPE]:
243+ pass
244+ else:
245+ raise
246+
247+
248+def _flush_trace():
249+ # run from atexit hook
250+ global _trace_file
251+ if _trace_file:
252+ _trace_file.flush()