Merge lp:~vila/bzr/917733-cmdline-overrides into lp:bzr/2.5

Proposed by Vincent Ladeuil
Status: Merged
Approved by: Martin Packman
Approved revision: no longer in the source branch.
Merged at revision: 6445
Proposed branch: lp:~vila/bzr/917733-cmdline-overrides
Merge into: lp:bzr/2.5
Diff against target: 40 lines (+11/-2)
2 files modified
bzrlib/commands.py (+8/-2)
doc/en/release-notes/bzr-2.5.txt (+3/-0)
To merge this branch: bzr merge lp:~vila/bzr/917733-cmdline-overrides
Reviewer Review Type Date Requested Status
Martin Packman (community) Approve
Review via email: mp+89216@code.launchpad.net

Commit message

Support scripts that don't call bzrlib.initialize() but still call run_bzr()

Description of the change

Low-risk fix for scripts that, despite calling run_bzr, don't call
bzrlib.intialize().

The long term fix is to properly support this use case which is more
invasive and may require discussion. I've discussed a plan at the rally last
week to get there and will work on that shortly (I need some support from
library_state too for caching local config files targetting a fix for
http://pad.lv/832042).

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

Tested manually with:

#!/usr/bin/python

import bzrlib
from bzrlib import commands

commands._register_builtin_commands()
commands.install_bzr_command_hooks()
import pdb; pdb.set_trace()
commands.run_bzr(['rocks'])

Traceback without the fix, works with it.

No output as the default UI is SilentUIFactory (tarmac uses a more complex setup but will use the same code path in the end).

Revision history for this message
Martin Packman (gz) wrote :

Looks fine, I'd add a comment before the `global_state is None` check saying what's up.

         # Reset the overrides
- bzrlib.global_state.cmdline_overrides._reset()
+ cmdline_overrides._reset()

Why is this here, rather than in LibraryState.__exit__? If it's not intended to have the same lifetime as the process, it shouldn't be using global_state at all.

review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

> Looks fine, I'd add a comment before the `global_state is None` check saying
> what's up.

I'll do.

>
> # Reset the overrides
> - bzrlib.global_state.cmdline_overrides._reset()
> + cmdline_overrides._reset()
>
> Why is this here, rather than in LibraryState.__exit__? If it's not intended
> to have the same lifetime as the process, it shouldn't be using global_state
> at all.

Mostly because we should *receive* a library state object there. But this requires a bit more care and will be more invasive.

Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/commands.py'
2--- bzrlib/commands.py 2011-12-18 12:46:49 +0000
3+++ bzrlib/commands.py 2012-01-19 11:31:24 +0000
4@@ -1071,7 +1071,13 @@
5 argv_copy.append(a)
6 i += 1
7
8- bzrlib.global_state.cmdline_overrides._from_cmdline(override_config)
9+ if bzrlib.global_state is None:
10+ # FIXME: Workaround for users that imported bzrlib but didn't call
11+ # bzrlib.initialize -- vila 2012-01-19
12+ cmdline_overrides = config.CommandLineStore()
13+ else:
14+ cmdline_overrides = bzrlib.global_state.cmdline_overrides
15+ cmdline_overrides._from_cmdline(override_config)
16
17 debug.set_debug_flags_from_config()
18
19@@ -1131,7 +1137,7 @@
20 trace.debug_memory('Process status after command:', short=False)
21 option._verbosity_level = saved_verbosity_level
22 # Reset the overrides
23- bzrlib.global_state.cmdline_overrides._reset()
24+ cmdline_overrides._reset()
25
26
27 def display_command(func):
28
29=== modified file 'doc/en/release-notes/bzr-2.5.txt'
30--- doc/en/release-notes/bzr-2.5.txt 2012-01-18 10:42:07 +0000
31+++ doc/en/release-notes/bzr-2.5.txt 2012-01-19 11:31:24 +0000
32@@ -32,6 +32,9 @@
33 .. Fixes for situations where bzr would previously crash or give incorrect
34 or undesirable results.
35
36+* Support scripts that don't call bzrlib.initialize() but still call run_bzr().
37+ (Vincent Ladeuil, #917733)
38+
39 * Test for equality instead of object identity where ROOT_PARENT is concerned.
40 (Wouter van Heyst, #881142)
41

Subscribers

People subscribed via source and target branches