Merge lp:~bialix/bzr/2.3-bugfix-660174 into lp:bzr/2.3

[Alexander Belchenko]

Bug #660174, trap accidental import of pwd on Windows where it doesn't exist.

* Don't fail with traceback if `bzr serve` is running as a service on Windows,
  and there is no USERNAME, nor BZR_EMAIL or other whoami-related environment
  variables set. (Alexander Belchenko, Bug #660174)

[Alexander Belchenko (bialix)]

Should I provide a patch against trunk/2.4 or that one will be automatically merged to trunk and 2.4?

[John A Meinel (jameinel)]

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

On 6/15/2011 4:08 PM, Alexander Belchenko wrote:
> Alexander Belchenko has proposed merging lp:~bialix/bzr/2.3-bugfix-660174 into lp:bzr/2.3.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #660174 in Bazaar: "locking a branch should not try to import pwd when running as a windows service"
> https://bugs.launchpad.net/bzr/+bug/660174
>
> For more details, see:
> https://code.launchpad.net/~bialix/bzr/2.3-bugfix-660174/+merge/64692
>
> * Don't fail with traceback if `bzr serve` is running as a service on Windows,
> and there is no USERNAME, nor BZR_EMAIL or other whoami-related environment
> variables set. (Alexander Belchenko, Bug #660174)
>

Anything in an earlier release will trickle into trunk, so you don't
have to be explicit.

> + # We should not fail with taceback in this case.
"traceback"

In the test, I would make sure all the other env vars are also removed,
so the test doesn't fail spuriously.

You should use "self.overrideEnv" rather than "set_or_unset_env". So you
don't have to restore the variable yourself.
    for name in ('LOGNAME', 'USER', 'LNAME', 'USERNAME'):
        self.overrideEnv(name, None)

 review: needsfixing

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

iEYEARECAAYFAk34x9wACgkQJdeBCYSNAAP/5ACgqCGj3Io7XYxWl0YMu7eaqOXM
pl4An0JRNv8rOWQO2XJ0DrQb5j7XcAge
=jBCc
-----END PGP SIGNATURE-----

review: Needs Fixing

[Alexander Belchenko (bialix)]

John A Meinel пишет:
> In the test, I would make sure all the other env vars are also removed,
> so the test doesn't fail spuriously.
>
> You should use "self.overrideEnv" rather than "set_or_unset_env". So you
> don't have to restore the variable yourself.
> for name in ('LOGNAME', 'USER', 'LNAME', 'USERNAME'):
> self.overrideEnv(name, None)
>
> review: needsfixing

overrideEnv() is nice, and it's much better indeed. Thanks for pointing
it out.

fixed and pushed.

[John A Meinel (jameinel)]

review: Approve

[John A Meinel (jameinel)]

sent to pqm by email

=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py	2011-01-12 21:06:32 +0000
+++ bzrlib/osutils.py	2011-06-15 15:23:41 +0000
@@ -2369,6 +2369,18 @@
     except UnicodeDecodeError:
         raise errors.BzrError("Can't decode username as %s." % \
                 user_encoding)
+    except ImportError, e:
+        if sys.platform != 'win32':
+            raise
+        if str(e) != 'No module named pwd':
+            raise
+        # https://bugs.launchpad.net/bzr/+bug/660174
+        # getpass.getuser() is unable to return username on Windows
+        # if there is no USERNAME environment variable set.
+        # That could be true if bzr is running as a service,
+        # e.g. running `bzr serve` as a service on Windows.
+        # We should not fail with traceback in this case.
+        username = u'UNKNOWN'
     return username
 
 

=== modified file 'bzrlib/tests/features.py'
--- bzrlib/tests/features.py	2011-01-12 01:01:53 +0000
+++ bzrlib/tests/features.py	2011-06-15 15:23:41 +0000
@@ -18,6 +18,7 @@
 
 import os
 import stat
+import sys
 
 from bzrlib import tests
 
@@ -131,3 +132,17 @@
 bash_feature = ExecutableFeature('bash')
 sed_feature = ExecutableFeature('sed')
 diff_feature = ExecutableFeature('diff')
+
+
+class Win32Feature(tests.Feature):
+    """Feature testing whether we're running selftest on Windows
+    or Windows-like platform.
+    """
+
+    def _probe(self):
+        return sys.platform == 'win32'
+
+    def feature_name(self):
+        return "win32 platform"
+
+win32_feature = Win32Feature()

=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py	2011-05-13 11:34:44 +0000
+++ bzrlib/tests/test_osutils.py	2011-06-15 15:23:41 +0000
@@ -2036,6 +2036,7 @@
         # Whatever the result is, if we don't raise an exception, it's ok.
         osutils.terminal_width()
 
+
 class TestCreationOps(tests.TestCaseInTempDir):
     _test_needs_features = [features.chown_feature]
 
@@ -2071,6 +2072,7 @@
         self.assertEquals(self.uid, s.st_uid)
         self.assertEquals(self.gid, s.st_gid)
 
+
 class TestGetuserUnicode(tests.TestCase):
 
     def test_ascii_user(self):
@@ -2091,6 +2093,13 @@
         self.overrideEnv('LOGNAME', u'jrandom\xb6'.encode(ue))
         self.assertEqual(u'jrandom\xb6', osutils.getuser_unicode())
 
+    def test_no_username_bug_660174(self):
+        self.requireFeature(features.win32_feature)
+        for name in ('LOGNAME', 'USER', 'LNAME', 'USERNAME'):
+            self.overrideEnv(name, None)
+        self.assertEqual(u'UNKNOWN', osutils.getuser_unicode())
+
+
 class TestBackupNames(tests.TestCase):
 
     def setUp(self):

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- doc/en/release-notes/bzr-2.3.txt	2011-06-09 12:17:09 +0000
+++ doc/en/release-notes/bzr-2.3.txt	2011-06-15 15:23:41 +0000
@@ -33,6 +33,10 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Don't fail with traceback if `bzr serve` is running as a service on Windows,
+  and there is no USERNAME, nor BZR_EMAIL or other whoami-related environment
+  variables set. (Alexander Belchenko, Bug #660174)
+
 Documentation
 *************