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

[Vincent Ladeuil]

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

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).

[Vincent Ladeuil (vila)]

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).

[Martin Packman (gz)]

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

[Vincent Ladeuil (vila)]

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

[Vincent Ladeuil (vila)]

sent to pqm by email

[lp:~vila/bzr/917733-cmdline-overrides]

6446. By Vincent Ladeuil on 2012-01-19

Clarifying why we need to workaround bzrlib.initialize() not being called.

=== modified file 'bzrlib/commands.py'
--- bzrlib/commands.py	2011-12-18 12:46:49 +0000
+++ bzrlib/commands.py	2012-01-19 11:31:24 +0000
@@ -1071,7 +1071,13 @@
             argv_copy.append(a)
         i += 1
 
-    bzrlib.global_state.cmdline_overrides._from_cmdline(override_config)
+    if bzrlib.global_state is None:
+        # FIXME: Workaround for users that imported bzrlib but didn't call
+        # bzrlib.initialize -- vila 2012-01-19
+        cmdline_overrides = config.CommandLineStore()
+    else:
+        cmdline_overrides = bzrlib.global_state.cmdline_overrides
+    cmdline_overrides._from_cmdline(override_config)
 
     debug.set_debug_flags_from_config()
 
@@ -1131,7 +1137,7 @@
             trace.debug_memory('Process status after command:', short=False)
         option._verbosity_level = saved_verbosity_level
         # Reset the overrides 
-        bzrlib.global_state.cmdline_overrides._reset()
+        cmdline_overrides._reset()
 
 
 def display_command(func):

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-01-18 10:42:07 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-01-19 11:31:24 +0000
@@ -32,6 +32,9 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Support scripts that don't call bzrlib.initialize() but still call run_bzr().
+  (Vincent Ladeuil, #917733)
+
 * Test for equality instead of object identity where ROOT_PARENT is concerned.
   (Wouter van Heyst, #881142)