Rev 4720: (jam) Give a clear error when we fail to open ~/.bzr.log, in file:///home/pqm/archives/thelove/bzr/2.0/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/2.0/

------------------------------------------------------------
revno: 4720 [merge]
revision-id: pqm at pqm.ubuntu.com-20100107085714-bfuh2lmk855sjexc
parent: pqm at pqm.ubuntu.com-20100106184401-mrv30dls621g1zq7
parent: john at arbash-meinel.com-20100106174615-cq1nckxhbuyemgjx
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.0
timestamp: Thu 2010-01-07 08:57:14 +0000
message:
  (jam) Give a clear error when we fail to open ~/.bzr.log,
  	instead of 'no handlers for "bzr"' (#503886).
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/test_trace.py     testtrace.py-20051110225523-a21117fc7a07eeff
  bzrlib/trace.py                trace.py-20050309040759-c8ed824bdcd4748a
=== modified file 'NEWS'
--- a/NEWS	2010-01-06 18:44:01 +0000
+++ b/NEWS	2010-01-07 08:57:14 +0000
@@ -38,6 +38,12 @@
 * Give a clearer message if the lockdir disappears after being apparently
   successfully taken.  (Martin Pool, #498378)
 
+* If we fail to open ``~/.bzr.log`` write a clear message to stderr rather
+  than using ``warning()``. The log file is opened before logging is set
+  up, and it leads to very confusing: 'no handlers for "bzr"' messages for
+  users, rather than something nicer.
+  (John Arbash Meinel, Barry Warsaw, #503886)
+
 * The 2a format wasn't properly restarting autopacks when something
   changed underneath it (like another autopack). Now concurrent
   autopackers will properly succeed. (John Arbash Meinel, #495000)

=== modified file 'bzrlib/tests/test_trace.py'
--- a/bzrlib/tests/test_trace.py	2009-09-03 02:59:56 +0000
+++ b/bzrlib/tests/test_trace.py	2010-01-06 17:46:15 +0000
@@ -27,6 +27,7 @@
 
 from bzrlib import (
     errors,
+    trace,
     )
 from bzrlib.tests import TestCaseInTempDir, TestCase
 from bzrlib.trace import (
@@ -238,6 +239,20 @@
             tmp1.close()
             tmp2.close()
 
+    def test__open_bzr_log_uses_stderr_for_failures(self):
+        # If _open_bzr_log cannot open the file, then we should write the
+        # warning to stderr. Since this is normally happening before logging is
+        # set up.
+        self.addCleanup(setattr, sys, 'stderr', sys.stderr)
+        self.addCleanup(setattr, trace, '_bzr_log_filename',
+                        trace._bzr_log_filename)
+        sys.stderr = StringIO()
+        # Set the log file to something that cannot exist
+        os.environ['BZR_LOG'] = os.getcwd() + '/no-dir/bzr.log'
+        logf = trace._open_bzr_log()
+        self.assertIs(None, logf)
+        self.assertContainsRe(sys.stderr.getvalue(),
+                              'failed to open trace file: .*/no-dir/bzr.log')
 
 class TestVerbosityLevel(TestCase):
 

=== modified file 'bzrlib/trace.py'
--- a/bzrlib/trace.py	2009-09-03 02:59:56 +0000
+++ b/bzrlib/trace.py	2010-01-06 17:46:15 +0000
@@ -237,7 +237,12 @@
             bzr_log_file.write("bug reports to https://bugs.launchpad.net/bzr/+filebug\n\n")
         return bzr_log_file
     except IOError, e:
-        warning("failed to open trace file: %s" % (e))
+        # If we are failing to open the log, then most likely logging has not
+        # been set up yet. So we just write to stderr rather than using
+        # 'warning()'. If we using warning(), users get the unhelpful 'no
+        # handlers registered for "bzr"' when something goes wrong on the
+        # server. (bug #503886)
+        sys.stderr.write("failed to open trace file: %s\n" % (e,))
     # TODO: What should happen if we fail to open the trace file?  Maybe the
     # objects should be pointed at /dev/null or the equivalent?  Currently
     # returns None which will cause failures later.