Fixed #21732 -- Made compilemessages complain about non-writable location

Instead of crashing with a CommandError, now a non-writable location
of mo files will only make compilemessages complain and continue.
Thanks Ramiro Morales for the review.

4 files changed, 111 insertions, 40 deletions

diff --git a/django/core/management/commands/compilemessages.py b/django/core/management/commands/compilemessages.py
index 4367c31a9f..0e55a28888 100644
--- a/django/core/management/commands/compilemessages.py
+++ b/django/core/management/commands/compilemessages.py
@@ -17,45 +17,15 @@ def has_bom(fn):
         sample.startswith(codecs.BOM_UTF16_BE)
 
 
-def compile_messages(stdout, locale=None):
-    program = 'msgfmt'
-    if find_command(program) is None:
-        raise CommandError("Can't find %s. Make sure you have GNU gettext tools 0.15 or newer installed." % program)
-
-    basedirs = [os.path.join('conf', 'locale'), 'locale']
-    if os.environ.get('DJANGO_SETTINGS_MODULE'):
-        from django.conf import settings
-        basedirs.extend([upath(path) for path in settings.LOCALE_PATHS])
-
-    # Gather existing directories.
-    basedirs = set(map(os.path.abspath, filter(os.path.isdir, basedirs)))
-
-    if not basedirs:
-        raise CommandError("This script should be run from the Django Git checkout or your project or app tree, or with the settings module specified.")
-
-    for basedir in basedirs:
-        if locale:
-            dirs = [os.path.join(basedir, l, 'LC_MESSAGES') for l in locale]
-        else:
-            dirs = [basedir]
-        for ldir in dirs:
-            for dirpath, dirnames, filenames in os.walk(ldir):
-                for f in filenames:
-                    if not f.endswith('.po'):
-                        continue
-                    stdout.write('processing file %s in %s\n' % (f, dirpath))
-                    fn = os.path.join(dirpath, f)
-                    if has_bom(fn):
-                        raise CommandError("The %s file has a BOM (Byte Order Mark). Django only supports .po files encoded in UTF-8 and without any BOM." % fn)
-                    pf = os.path.splitext(fn)[0]
-                    args = [program, '--check-format', '-o', npath(pf + '.mo'), npath(pf + '.po')]
-                    output, errors, status = popen_wrapper(args)
-                    if status:
-                        if errors:
-                            msg = "Execution of %s failed: %s" % (program, errors)
-                        else:
-                            msg = "Execution of %s failed" % program
-                        raise CommandError(msg)
+def is_writable(path):
+    # Known side effect: updating file access/modified time to current time if
+    # it is writable.
+    try:
+        with open(path, 'a'):
+            os.utime(path, None)
+    except (IOError, OSError):
+        return False
+    return True
 
 
 class Command(BaseCommand):
@@ -67,7 +37,67 @@ class Command(BaseCommand):
 
     requires_system_checks = False
     leave_locale_alone = True
+    program = 'msgfmt'
 
     def handle(self, **options):
         locale = options.get('locale')
-        compile_messages(self.stdout, locale=locale)
+        self.verbosity = int(options.get('verbosity'))
+
+        if find_command(self.program) is None:
+            raise CommandError("Can't find %s. Make sure you have GNU gettext "
+                               "tools 0.15 or newer installed." % self.program)
+
+        basedirs = [os.path.join('conf', 'locale'), 'locale']
+        if os.environ.get('DJANGO_SETTINGS_MODULE'):
+            from django.conf import settings
+            basedirs.extend([upath(path) for path in settings.LOCALE_PATHS])
+
+        # Gather existing directories.
+        basedirs = set(map(os.path.abspath, filter(os.path.isdir, basedirs)))
+
+        if not basedirs:
+            raise CommandError("This script should be run from the Django Git "
+                               "checkout or your project or app tree, or with "
+                               "the settings module specified.")
+
+        for basedir in basedirs:
+            if locale:
+                dirs = [os.path.join(basedir, l, 'LC_MESSAGES') for l in locale]
+            else:
+                dirs = [basedir]
+            locations = []
+            for ldir in dirs:
+                for dirpath, dirnames, filenames in os.walk(ldir):
+                    locations.extend((dirpath, f) for f in filenames if f.endswith('.po'))
+            if locations:
+                self.compile_messages(locations)
+
+    def compile_messages(self, locations):
+        """
+        Locations is a list of tuples: [(directory, file), ...]
+        """
+        for i, (dirpath, f) in enumerate(locations):
+            if self.verbosity > 0:
+                self.stdout.write('processing file %s in %s\n' % (f, dirpath))
+            po_path = os.path.join(dirpath, f)
+            if has_bom(po_path):
+                raise CommandError("The %s file has a BOM (Byte Order Mark). "
+                                   "Django only supports .po files encoded in "
+                                   "UTF-8 and without any BOM." % po_path)
+            base_path = os.path.splitext(po_path)[0]
+
+            # Check writability on first location
+            if i == 0 and not is_writable(npath(base_path + '.mo')):
+                self.stderr.write("The po files under %s are in a seemingly not "
+                                  "writable location. mo files will not be updated/created." % dirpath)
+                return
+
+            args = [self.program, '--check-format', '-o',
+                    npath(base_path + '.mo'), npath(base_path + '.po')]
+            output, errors, status = popen_wrapper(args)
+            if status:
+                if errors:
+                    msg = "Execution of %s failed: %s" % (self.program, errors)
+                else:
+                    msg = "Execution of %s failed" % self.program
+                raise CommandError(msg)
diff --git a/tests/i18n/commands/locale/en/LC_MESSAGES/django.mo b/tests/i18n/commands/locale/en/LC_MESSAGES/django.mo
new file mode 100644
index 0000000000..b79b39100b
--- /dev/null
+++ b/tests/i18n/commands/locale/en/LC_MESSAGES/django.mo
diff --git a/tests/i18n/commands/locale/en/LC_MESSAGES/django.po b/tests/i18n/commands/locale/en/LC_MESSAGES/django.po
new file mode 100644
index 0000000000..ddb831b24b
--- /dev/null
+++ b/tests/i18n/commands/locale/en/LC_MESSAGES/django.po
@@ -0,0 +1,27 @@
+# SOME DESCRIPTIVE TITLE.
+# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
+# This file is distributed under the same license as the PACKAGE package.
+# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
+#
+#, fuzzy
+msgid ""
+msgstr ""
+"Project-Id-Version: PACKAGE VERSION\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2014-01-04 22:05+0100\n"
+"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
+"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
+"Language-Team: LANGUAGE <LL@li.org>\n"
+"Language: \n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+
+#. Translators: This comment should be extracted
+#: __init__.py:4
+msgid "This is a translatable string."
+msgstr ""
+
+#: __init__.py:7
+msgid "This is another translatable string."
+msgstr ""
diff --git a/tests/i18n/test_compilation.py b/tests/i18n/test_compilation.py
index ae92f6f368..463ff21d23 100644
--- a/tests/i18n/test_compilation.py
+++ b/tests/i18n/test_compilation.py
@@ -1,4 +1,5 @@
 import os
+import stat
 import unittest
 
 from django.core.management import call_command, CommandError
@@ -37,6 +38,19 @@ class PoFileTests(MessageCompilationTests):
         self.assertIn("file has a BOM (Byte Order Mark)", cm.exception.args[0])
         self.assertFalse(os.path.exists(self.MO_FILE))
 
+    def test_no_write_access(self):
+        mo_file_en = 'locale/en/LC_MESSAGES/django.mo'
+        err_buffer = StringIO()
+        # put file in read-only mode
+        old_mode = os.stat(mo_file_en).st_mode
+        os.chmod(mo_file_en, stat.S_IREAD)
+        try:
+            call_command('compilemessages', locale=['en'], stderr=err_buffer, verbosity=0)
+            err = err_buffer.getvalue()
+            self.assertIn("not writable location", err)
+        finally:
+            os.chmod(mo_file_en, old_mode)
+
 
 class PoFileContentsTests(MessageCompilationTests):
     # Ticket #11240