Merge lp:~hloeung/bzr/mailname-first-line into lp:bzr

[Haw Loeung]

Read only the first line from /etc/mailname. This makes it consistent with postfix and allows us to append our puppet warning headers.

[Jelmer Vernooij (jelmer)]

Thanks for the fix; the change looks reasonable. It would be good to have a test for it so we can be sure this won't regress in the future.

[Haw Loeung (hloeung)]

Hi Jelmer,

I have added a couple of test cases. This is my first so all feedback appreciated.

Regards,

Haw

review: Needs Resubmitting

[Haw Loeung (hloeung)]

Err, discard the "resubmit".

[Jelmer Vernooij (jelmer)]

Thanks, looks great!

One really minor nitpick: we should use try/finally to make sure the tests don't leak any file descriptors if they fail. It would also be nice to have a news entry about this in doc/en/release-notes/bzr-2.6.txt

review: Approve

[Haw Loeung (hloeung)]

Jelmer,

I've updated the release notes for 2.6 and made the tests use try/finally to make sure we don't leak any file descriptors if/when they fail. How does it look now?

Thanks for your feedback.

[Haw Loeung (hloeung)]

review: Needs Information

[Haw Loeung (hloeung)]

[hloeung@darkon mailname-first-line]$ ./bzr selftest -v TestDefaultMailDomain
running 3 tests...
bzr selftest: /home/hloeung/tmp/mailname-first-line/bzr
   /home/hloeung/tmp/mailname-first-line/bzrlib
   bzr-2.6.0dev3 python-2.7.3 Linux-3.5.0-5-generic-x86_64-with-Ubuntu-12.10-quantal

...stDefaultMailDomain.test_default_mail_domain_multiple_lines OK 21ms
...onfig.TestDefaultMailDomain.test_default_mail_domain_no_eol OK 2ms
...onfig.TestDefaultMailDomain.test_default_mail_domain_simple OK 2ms
----------------------------------------------------------------------
Ran 3 tests in 0.046s

OK

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2012-06-19 16:36:07 +0000
+++ bzrlib/config.py	2012-07-21 02:03:18 +0000
@@ -1562,7 +1562,7 @@
         return os.path.expanduser('~/.cache')
 
 
-def _get_default_mail_domain():
+def _get_default_mail_domain(mailname_file='/etc/mailname'):
     """If possible, return the assumed default email domain.
 
     :returns: string mail domain, or None.
@@ -1571,11 +1571,11 @@
         # No implementation yet; patches welcome
         return None
     try:
-        f = open('/etc/mailname')
+        f = open(mailname_file)
     except (IOError, OSError), e:
         return None
     try:
-        domain = f.read().strip()
+        domain = f.readline().strip()
         return domain
     finally:
         f.close()

=== modified file 'bzrlib/tests/test_config.py'
--- bzrlib/tests/test_config.py	2012-06-20 14:35:26 +0000
+++ bzrlib/tests/test_config.py	2012-07-21 02:03:18 +0000
@@ -4568,8 +4568,8 @@
                                            port=99, path='/foo',
                                            realm='realm')
         CREDENTIALS = {'name': 'name', 'user': 'user', 'password': 'password',
-                       'verify_certificates': False, 'scheme': 'scheme', 
-                       'host': 'host', 'port': 99, 'path': '/foo', 
+                       'verify_certificates': False, 'scheme': 'scheme',
+                       'host': 'host', 'port': 99, 'path': '/foo',
                        'realm': 'realm'}
         self.assertEqual(CREDENTIALS, credentials)
         credentials_from_disk = config.AuthenticationConfig().get_credentials(
@@ -4583,8 +4583,8 @@
         self.assertIs(None, conf._get_config().get('name'))
         credentials = conf.get_credentials(host='host', scheme='scheme')
         CREDENTIALS = {'name': 'name2', 'user': 'user2', 'password':
-                       'password', 'verify_certificates': True, 
-                       'scheme': 'scheme', 'host': 'host', 'port': None, 
+                       'password', 'verify_certificates': True,
+                       'scheme': 'scheme', 'host': 'host', 'port': None,
                        'path': None, 'realm': None}
         self.assertEqual(CREDENTIALS, credentials)
 
@@ -4876,6 +4876,37 @@
             self.assertEquals((None, None), (realname, address))
 
 
+class TestDefaultMailDomain(tests.TestCaseInTempDir):
+    """Test retrieving default domain from mailname file"""
+
+    def test_default_mail_domain_simple(self):
+        f = file('simple', 'w')
+        try:
+            f.write("domainname.com\n")
+        finally:
+            f.close()
+        r = config._get_default_mail_domain('simple')
+        self.assertEquals('domainname.com', r)
+
+    def test_default_mail_domain_no_eol(self):
+        f = file('no_eol', 'w')
+        try:
+            f.write("domainname.com")
+        finally:
+            f.close()
+        r = config._get_default_mail_domain('no_eol')
+        self.assertEquals('domainname.com', r)
+
+    def test_default_mail_domain_multiple_lines(self):
+        f = file('multiple_lines', 'w')
+        try:
+            f.write("domainname.com\nsome other text\n")
+        finally:
+            f.close()
+        r = config._get_default_mail_domain('multiple_lines')
+        self.assertEquals('domainname.com', r)
+
+
 class EmailOptionTests(tests.TestCase):
 
     def test_default_email_uses_BZR_EMAIL(self):

=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt	2012-07-10 12:19:23 +0000
+++ doc/en/release-notes/bzr-2.6.txt	2012-07-21 02:03:18 +0000
@@ -23,7 +23,7 @@
 Improvements
 ************
 
-.. Improvements to existing commands, especially improved performance 
+.. Improvements to existing commands, especially improved performance
    or memory usage, or better results.
 
 Bug Fixes
@@ -32,6 +32,11 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Fixed a bug where the entire contents of ``/etc/mailname`` is read in.
+  We only want to read in the first line so that comments could be added
+  and would be ignored.
+  (Haw Loeung, #932515)
+
 Documentation
 *************