Merge lp:~gz/bzr/non_ascii_bzr_log_312841 into lp:bzr/2.5

[Martin Packman]

Store the path to .bzr.log as unicode so it can be safely printed by `bzr version`

Fixes a UnicodeDecodeError where someone has a non-ascii home dir or sets one of the variables used to influence the location of .bzr.log to a non-ascii value. As per other changes along these, the trade off is that someone who sets BZR_LOG on a posix system to some random bytestring not in their locale will now get an error. That's a less likely scenario and can't be fixed without some bigger changes to how bzr deals with paths.

[Jelmer Vernooij (jelmer)]

On Wed, Feb 01, 2012 at 01:49:33PM -0000, Martin Packman wrote:
> Martin Packman has proposed merging lp:~gz/bzr/non_ascii_bzr_log_312841 into lp:bzr/2.5 with lp:~gz/bzr/get_home_dir as a prerequisite.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #312841 in Bazaar: "UnicodeDecodeError from bzr version when log file has non-ascii path"
> https://bugs.launchpad.net/bzr/+bug/312841
>
> For more details, see:
> https://code.launchpad.net/~gz/bzr/non_ascii_bzr_log_312841/+merge/91076
>
> Fixes a UnicodeDecodeError where someone has a non-ascii home dir or sets one of the variables used to influence the location of .bzr.log to a non-ascii value. As per other changes along these, the trade off is that someone who sets BZR_LOG on a posix system to some random bytestring not in their locale will now get an error. That's a less likely scenario and can't be fixed without some bigger changes to how bzr deals with paths.

I'm sure there's a unicode symbol symbol expressing approval
somewhere, but I can't find it at the moment. For now, then:

  merge approve

Cheers,

Jelmer

review: Approve

[Martin Packman (gz)]

sent to pqm by email

=== modified file 'bzrlib/tests/blackbox/test_version.py'
--- bzrlib/tests/blackbox/test_version.py	2011-05-13 12:51:05 +0000
+++ bzrlib/tests/blackbox/test_version.py	2012-02-01 17:15:21 +0000
@@ -129,3 +129,20 @@
         self.assertTrue(len(out) > 0)
         self.assertContainsRe(out, r"(?m)^  Bazaar log file: " + bzr_log)
         self.assertPathDoesNotExist(default_log)
+
+    def test_unicode_bzr_log(self):
+        uni_val = u"\xa7"
+        enc = osutils.get_user_encoding()
+        try:
+            str_val = uni_val.encode(enc)
+        except UnicodeEncodeError:
+            self.skip("Test string %r unrepresentable in user encoding %s" % (
+                uni_val, enc))
+        self.overrideEnv('BZR_HOME', self.test_base_dir)
+        self.overrideEnv("BZR_LOG",
+            os.path.join(self.test_base_dir, uni_val).encode(enc))
+        out, err = self.run_bzr_subprocess("version")
+        uni_out = out.decode(enc)
+        self.assertContainsRe(uni_out, u"(?m)^  Bazaar log file: .*/\xa7$")
+
+

=== modified file 'bzrlib/trace.py'
--- bzrlib/trace.py	2011-12-19 13:23:58 +0000
+++ bzrlib/trace.py	2012-02-01 17:15:21 +0000
@@ -194,16 +194,15 @@
 
 
 def _get_bzr_log_filename():
-    bzr_log = os.environ.get('BZR_LOG')
+    bzr_log = osutils.path_from_environ('BZR_LOG')
     if bzr_log:
         return bzr_log
-    home = os.environ.get('BZR_HOME')
+    home = osutils.path_from_environ('BZR_HOME')
     if home is None:
-        if sys.platform == 'win32':
-            from bzrlib import win32utils
-            home = win32utils.get_home_location()
-        else:
-            home = os.path.expanduser('~')
+        # GZ 2012-02-01: Logging to the home dir is bad, but XDG is unclear
+        #                over what would be better. On windows, bug 240550
+        #                suggests LOCALAPPDATA be used instead.
+        home = osutils._get_home_dir()
     return os.path.join(home, '.bzr.log')
 
 

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-01-31 18:25:57 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-02-01 17:15:21 +0000
@@ -66,6 +66,9 @@
 * ``bzr send`` works on treeless branches again.
   (Jelmer Vernooij, #921591)
 
+* ``bzr version`` no longer throws a UnicodeDecodeError if the .bzr.log path
+  contains non-ascii characters. (Martin Packman, #312841)
+
 * Support scripts that don't call bzrlib.initialize() but still call run_bzr().
   (Vincent Ladeuil, #917733)