Merge lp:~mbp/bzr/whoami into lp:bzr/2.3

[Martin Pool]

infer default user name from /etc/mailname on unix (bug 616878)

This addresses bug 616878 by making bzr automatically infer an email address if /etc/mailname is set. This tries to get a balance between on the one hand saving people from committing lots of work as joe@localhost, and on the other hand letting them commit without needing manual setup. Basically it's a partial reversion and then readjustment for the fix for bug 549310.

[Vincent Ladeuil (vila)]

You just got a lot of positive karma from admins all over the planet ;-D

94 + # we try utf-8 first, because on many variants (like Linux),
95 + # /etc/passwd "should" be in utf-8, and because it's unlikely to give
96 + # false positives. (many users will have their user encoding set to
97 + # latin-1, which cannot raise UnicodeError.)

You will make savannah users happy with that, I think there is even a bug for that https://bugs.launchpad.net/bzr/+bug/659763 .

I think you could easily add more tests for _auto_user_id() and _get_default_mail_domain() by adding optional parameters to inject whatever test files you need (/etc/passwd (or getuid(), pwd.getpwuid() whatever), /etc/mailname, etc)

That's a tweak

review: Needs Fixing

[Martin Pool (mbp)]

The unicode code is not originally mine; it's resurrected from I think
mgz. So if that bug existed before, it probably still exists with
this patch...

[Vincent Ladeuil (vila)]

@poolie: As I understood it from bug #659763, the issue was that we were using an ASCII encoding to interpret /etc/passwd instead of using utf-8, so I think your patch *addresses* the bug.

[Martin Pool (mbp)]

sent to pqm by email

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2011-01-12 18:12:58 +0000
+++ bzrlib/config.py	2011-04-01 03:15:39 +0000
@@ -63,6 +63,7 @@
 """
 
 import os
+import string
 import sys
 
 from bzrlib import commands
@@ -271,21 +272,21 @@
         the concrete policy type is checked, and finally
         $EMAIL is examined.
         If no username can be found, errors.NoWhoami exception is raised.
-
-        TODO: Check it's reasonably well-formed.
         """
         v = os.environ.get('BZR_EMAIL')
         if v:
             return v.decode(osutils.get_user_encoding())
-
         v = self._get_user_id()
         if v:
             return v
-
         v = os.environ.get('EMAIL')
         if v:
             return v.decode(osutils.get_user_encoding())
-
+        name, email = _auto_user_id()
+        if name and email:
+            return '%s <%s>' % (name, email)
+        elif email:
+            return email
         raise errors.NoWhoami()
 
     def ensure_username(self):
@@ -1213,6 +1214,86 @@
         return os.path.expanduser('~/.cache')
 
 
+def _get_default_mail_domain():
+    """If possible, return the assumed default email domain.
+
+    :returns: string mail domain, or None.
+    """
+    if sys.platform == 'win32':
+        # No implementation yet; patches welcome
+        return None
+    try:
+        f = open('/etc/mailname')
+    except (IOError, OSError), e:
+        return None
+    try:
+        domain = f.read().strip()
+        return domain
+    finally:
+        f.close()
+
+
+def _auto_user_id():
+    """Calculate automatic user identification.
+
+    :returns: (realname, email), either of which may be None if they can't be
+    determined.
+
+    Only used when none is set in the environment or the id file.
+
+    This only returns an email address if we can be fairly sure the 
+    address is reasonable, ie if /etc/mailname is set on unix.
+
+    This doesn't use the FQDN as the default domain because that may be 
+    slow, and it doesn't use the hostname alone because that's not normally 
+    a reasonable address.
+    """
+    if sys.platform == 'win32':
+        # No implementation to reliably determine Windows default mail
+        # address; please add one.
+        return None, None
+
+    default_mail_domain = _get_default_mail_domain()
+    if not default_mail_domain:
+        return None, None
+
+    import pwd
+    uid = os.getuid()
+    try:
+        w = pwd.getpwuid(uid)
+    except KeyError:
+        mutter('no passwd entry for uid %d?' % uid)
+        return None, None
+
+    # we try utf-8 first, because on many variants (like Linux),
+    # /etc/passwd "should" be in utf-8, and because it's unlikely to give
+    # false positives.  (many users will have their user encoding set to
+    # latin-1, which cannot raise UnicodeError.)
+    try:
+        gecos = w.pw_gecos.decode('utf-8')
+        encoding = 'utf-8'
+    except UnicodeError:
+        try:
+            encoding = osutils.get_user_encoding()
+            gecos = w.pw_gecos.decode(encoding)
+        except UnicodeError, e:
+            mutter("cannot decode passwd entry %s" % w)
+            return None, None
+    try:
+        username = w.pw_name.decode(encoding)
+    except UnicodeError, e:
+        mutter("cannot decode passwd entry %s" % w)
+        return None, None
+
+    comma = gecos.find(',')
+    if comma == -1:
+        realname = gecos
+    else:
+        realname = gecos[:comma]
+
+    return realname, (username + '@' + default_mail_domain)
+
+
 def parse_username(username):
     """Parse e-mail username and return a (name, address) tuple."""
     match = re.match(r'(.*?)\s*<?([\w+.-]+@[\w+.-]+)>?', username)

=== modified file 'bzrlib/tests/test_config.py'
--- bzrlib/tests/test_config.py	2011-02-22 09:49:45 +0000
+++ bzrlib/tests/test_config.py	2011-04-01 03:15:39 +0000
@@ -41,6 +41,7 @@
     )
 from bzrlib.tests import (
     features,
+    TestSkipped,
     scenarios,
     )
 from bzrlib.util.configobj import configobj
@@ -2237,3 +2238,25 @@
 # test_user_prompted ?
 class TestAuthenticationRing(tests.TestCaseWithTransport):
     pass
+
+
+class TestAutoUserId(tests.TestCase):
+    """Test inferring an automatic user name."""
+
+    def test_auto_user_id(self):
+        """Automatic inference of user name.
+        
+        This is a bit hard to test in an isolated way, because it depends on
+        system functions that go direct to /etc or perhaps somewhere else.
+        But it's reasonable to say that on Unix, with an /etc/mailname, we ought
+        to be able to choose a user name with no configuration.
+        """
+        if sys.platform == 'win32':
+            raise TestSkipped("User name inference not implemented on win32")
+        realname, address = config._auto_user_id()
+        if os.path.exists('/etc/mailname'):
+            self.assertTrue(realname)
+            self.assertTrue(address)
+        else:
+            self.assertEquals((None, None), (realname, address))
+

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- doc/en/release-notes/bzr-2.3.txt	2011-04-01 01:37:43 +0000
+++ doc/en/release-notes/bzr-2.3.txt	2011-04-01 03:15:39 +0000
@@ -39,6 +39,12 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Bazaar now infers the default user email address on Unix from the local
+  account name plus the contents of ``/etc/mailname`` if that file exists.
+  In particular, this means that committing as root through etckeeper will
+  normally not require running ``bzr whoami`` first.
+  (Martin Pool, #616878)
+
 * When reporting a crash without apport, don't print the full list of
    plugins because it's often too long.
    (Martin Pool, #716389)