refactor config file; fix duplicate merge

conf/locale/config

-{"locales" : ["en"]}
+{
+ "locales" : ["en"],
+ "dummy-locale" : "fr"
+}

i18n/config.py

+import os, json
+
+# BASE_DIR is the working directory to execute django-admin commands from.
+# Typically this should be the 'mitx' directory.
+BASE_DIR = os.path.normpath(os.path.dirname(os.path.abspath(__file__))+'/..')
+
+# LOCALE_DIR contains the locale files.
+# Typically this should be 'mitx/conf/locale'
+LOCALE_DIR =  os.path.join(BASE_DIR, 'conf', 'locale')
+
+class Configuration:
+    """
+    # Reads localization configuration in json format
+
+    """
+    _source_locale = 'en'
+
+    def __init__(self, filename):
+        self.filename = filename
+        self.config = self.get_config(self.filename)
+
+    def get_config(self, filename):
+        """
+        Returns data found in config file (as dict), or raises exception if file not found
+        """
+        if not os.path.exists(filename):
+            raise Exception("Configuration file cannot be found: %s" % filename)
+        with open(filename) as stream:
+            return json.load(stream)
+
+    def get_locales(self):
+        """
+        Returns a list of locales declared in the configuration file,
+        e.g. ['en', 'fr', 'es']
+        Each locale is a string.
+        """
+        return self.config['locales']
+
+    def get_source_locale(self):
+        """
+        Returns source language.
+        Source language is English.
+        """
+        return self._source_locale
+
+    def get_dummy_locale(self):
+        """
+        Returns a locale to use for the dummy text, e.g. 'fr'.
+        Throws exception if no dummy-locale is declared. 
+        The locale is a string.
+        """
+        dummy = self.config.get('dummy-locale', None)
+        if not dummy:
+            raise Exception('Could not read dummy-locale from configuration file.')
+        return dummy
+
+    def get_messages_dir(self, locale):
+        """
+        Returns the name of the directory holding the po files for locale.
+        Example: mitx/conf/locale/fr/LC_MESSAGES
+        """
+        return os.path.join(LOCALE_DIR, locale, 'LC_MESSAGES')
+
+    def get_source_messages_dir(self):
+        """
+        Returns the name of the directory holding the source-language po files (English).
+        Example: mitx/conf/locale/en/LC_MESSAGES
+        """
+        return self.get_messages_dir(self.get_source_locale())
+
+
+CONFIGURATION = Configuration(os.path.normpath(os.path.join(LOCALE_DIR, 'config')))
+

i18n/execute.py

-import os, subprocess, logging, json
+import os, subprocess, logging
+from config import CONFIGURATION, BASE_DIR
 
-def init_module():
-    """
-    Initializes module parameters
-    """
-    global BASE_DIR, LOCALE_DIR, CONFIG_FILENAME, SOURCE_MSGS_DIR, SOURCE_LOCALE, LOG
-
-    # BASE_DIR is the working directory to execute django-admin commands from.
-    # Typically this should be the 'mitx' directory.
-    BASE_DIR = os.path.normpath(os.path.dirname(os.path.abspath(__file__))+'/..')
-
-    # Source language is English
-    SOURCE_LOCALE = 'en'
-
-    # LOCALE_DIR contains the locale files.
-    # Typically this should be 'mitx/conf/locale'
-    LOCALE_DIR = BASE_DIR + '/conf/locale'
-
-    # CONFIG_FILENAME contains localization configuration in json format
-    CONFIG_FILENAME = LOCALE_DIR + '/config'
-
-    # SOURCE_MSGS_DIR contains the English po files.
-    SOURCE_MSGS_DIR = messages_dir(SOURCE_LOCALE)
-
-    # Default logger.
-    LOG = get_logger()
-    
-
-def messages_dir(locale):
-    """
-    Returns the name of the directory holding the po files for locale.
-    Example: mitx/conf/locale/en/LC_MESSAGES
-    """
-    return os.path.join(LOCALE_DIR, locale, 'LC_MESSAGES')
 
-def get_logger():
+def get_default_logger():
     """Returns a default logger"""
     log = logging.getLogger(__name__)
     log.setLevel(logging.INFO)
@@ -43,8 +11,8 @@ def get_logger():
     log.addHandler(log_handler)
     return log
 
-# Run this after defining messages_dir and get_logger, because it depends on these.
-init_module()    
+LOG = get_default_logger()
+
 
 def execute (command, working_directory=BASE_DIR, log=LOG):
     """
@@ -69,17 +37,6 @@ def call(command, working_directory=BASE_DIR, log=LOG):
     out, err = p.communicate()
     return (out, err)
     
-    
-def get_config():
-    """Returns data found in config file, or returns None if file not found"""
-    config_path = os.path.normpath(CONFIG_FILENAME)
-    if not os.path.exists(config_path):
-        log.warn("Configuration file cannot be found: %s" % \
-                 os.path.relpath(config_path, BASE_DIR))
-        return None
-    with open(config_path) as stream:
-        return json.load(stream)
-    
 def create_dir_if_necessary(pathname):
     dirname = os.path.dirname(pathname)
     if not os.path.exists(dirname):
@@ -98,4 +55,3 @@ def remove_file(filename, log=LOG, verbose=True):
         log.warn("File does not exist: %s" % os.path.relpath(filename, BASE_DIR))
     else:
         os.remove(filename)
-

i18n/extract.py

@@ -18,9 +18,8 @@ See https://edx-wiki.atlassian.net/wiki/display/ENG/PO+File+workflow
 import os
 from datetime import datetime
 from polib import pofile
-from execute import execute, create_dir_if_necessary, remove_file, \
-     BASE_DIR, LOCALE_DIR, SOURCE_MSGS_DIR, LOG
-
+from config import BASE_DIR, LOCALE_DIR, CONFIGURATION
+from execute import execute, create_dir_if_necessary, remove_file, LOG
 
 # BABEL_CONFIG contains declarations for Babel to extract strings from mako template files
 # Use relpath to reduce noise in logs
@@ -28,15 +27,19 @@ BABEL_CONFIG = os.path.relpath(LOCALE_DIR + '/babel.cfg', BASE_DIR)
 
 # Strings from mako template files are written to BABEL_OUT
 # Use relpath to reduce noise in logs
-BABEL_OUT = os.path.relpath(SOURCE_MSGS_DIR + '/mako.po', BASE_DIR)
+BABEL_OUT = os.path.relpath(CONFIGURATION.get_source_messages_dir() + '/mako.po', BASE_DIR)
 
+SOURCE_WARN = 'This English source file is machine-generated. Do not check it into github'
 
 def main ():
     create_dir_if_necessary(LOCALE_DIR)
-    generated_files = ('django-partial.po', 'djangojs.po', 'mako.po')
+    source_msgs_dir = CONFIGURATION.get_source_messages_dir()
 
+    remove_file(os.path.join(source_msgs_dir, 'django.po'))
+    generated_files = ('django-partial.po', 'djangojs.po', 'mako.po')
     for filename in generated_files:
-        remove_file(os.path.join(SOURCE_MSGS_DIR, filename))
+        remove_file(os.path.join(source_msgs_dir, filename))
+
 
     # Extract strings from mako templates
     babel_mako_cmd = 'pybabel extract -F %s -c "TRANSLATORS:" . -o %s' % (BABEL_CONFIG, BABEL_OUT)
@@ -52,13 +55,13 @@ def main ():
     execute(make_django_cmd, working_directory=BASE_DIR)
     # makemessages creates 'django.po'. This filename is hardcoded.
     # Rename it to django-partial.po to enable merging into django.po later.
-    os.rename(os.path.join(SOURCE_MSGS_DIR, 'django.po'), 
-              os.path.join(SOURCE_MSGS_DIR, 'django-partial.po'))
+    os.rename(os.path.join(source_msgs_dir, 'django.po'), 
+              os.path.join(source_msgs_dir, 'django-partial.po'))
     execute(make_djangojs_cmd, working_directory=BASE_DIR)
 
     for filename in generated_files:
         LOG.info('Cleaning %s' % filename)
-        po = pofile(os.path.join(SOURCE_MSGS_DIR, filename))
+        po = pofile(os.path.join(source_msgs_dir, filename))
         # replace default headers with edX headers
         fix_header(po)
         # replace default metadata with edX metadata
@@ -82,8 +85,8 @@ def fix_header(po):
     po.metadata_is_fuzzy = []   # remove [u'fuzzy']
     header = po.header
     fixes = (
-        ('SOME DESCRIPTIVE TITLE', 'edX translation file'),
-        ('Translations template for PROJECT.', 'edX translation file'),
+        ('SOME DESCRIPTIVE TITLE', 'edX translation file\n' + SOURCE_WARN),
+        ('Translations template for PROJECT.', 'edX translation file\n' + SOURCE_WARN),
         ('YEAR', '%s' % datetime.utcnow().year),
         ('ORGANIZATION', 'edX'),
         ("THE PACKAGE'S COPYRIGHT HOLDER", "EdX"),

i18n/generate.py

@@ -16,15 +16,15 @@
 import os
 from polib import pofile
 
-from execute import execute, get_config, messages_dir, remove_file, \
-     BASE_DIR, LOG, SOURCE_LOCALE
+from config import BASE_DIR, CONFIGURATION
+from execute import execute, remove_file, LOG
 
 def merge(locale, target='django.po'):
     """
     For the given locale, merge django-partial.po, messages.po, mako.po -> django.po
     """
     LOG.info('Merging locale={0}'.format(locale))
-    locale_directory = messages_dir(locale)
+    locale_directory = CONFIGURATION.get_messages_dir(locale)
     files_to_merge = ('django-partial.po', 'messages.po', 'mako.po')
     validate_files(locale_directory, files_to_merge)
 
@@ -62,15 +62,8 @@ def validate_files(dir, files_to_merge):
             raise Exception("File not found: {0}".format(pathname))
 
 def main ():
-    configuration = get_config()
-    if configuration == None:
-        LOG.warn('Configuration file not found, using only English.')
-        locales = (SOURCE_LOCALE,)
-    else:
-        locales = configuration['locales']
-    for locale in locales:
+    for locale in CONFIGURATION.get_locales():
         merge(locale)
-
     compile_cmd = 'django-admin.py compilemessages'
     execute(compile_cmd, working_directory=BASE_DIR)
 

i18n/tests/__init__.py

+from test_config import TestConfiguration
 from test_extract import TestExtract
 from test_generate import TestGenerate
 from test_converter import TestConverter

i18n/tests/test_config.py

+import os
+from unittest import TestCase
+
+from config import Configuration, LOCALE_DIR, CONFIGURATION
+
+class TestConfiguration(TestCase):
+    """
+    Tests functionality of i18n/config.py
+    """
+
+    def test_config(self):
+        config_filename = os.path.normpath(os.path.join(LOCALE_DIR, 'config'))
+        config = Configuration(config_filename)
+        self.assertEqual(config.get_source_locale(), 'en')
+
+    def test_no_config(self):
+        config_filename = os.path.normpath(os.path.join(LOCALE_DIR, 'no_such_file'))
+        with self.assertRaises(Exception):
+            Configuration(config_filename)
+            
+    def test_valid_configuration(self):
+        """
+        Make sure we have a valid configuration file,
+        and that it contains an 'en' locale.
+        Also check values of dummy_locale and source_locale.
+        """
+        self.assertIsNotNone(CONFIGURATION)
+        locales = CONFIGURATION.get_locales()
+        self.assertIsNotNone(locales)
+        self.assertIsInstance(locales, list)
+        self.assertIn('en', locales)
+        self.assertEqual('fr', CONFIGURATION.get_dummy_locale())
+        self.assertEqual('en', CONFIGURATION.get_source_locale())

i18n/tests/test_extract.py

@@ -4,7 +4,7 @@ from nose.plugins.skip import SkipTest
 from datetime import datetime, timedelta
 
 import extract
-from execute import SOURCE_MSGS_DIR
+from config import CONFIGURATION
 
 # Make sure setup runs only once
 SETUP_HAS_RUN = False
@@ -39,7 +39,7 @@ class TestExtract(TestCase):
         Fails assertion if one of the files doesn't exist.
         """
         for filename in self.generated_files:
-            path = os.path.join(SOURCE_MSGS_DIR, filename)
+            path = os.path.join(CONFIGURATION.get_source_messages_dir(), filename)
             exists = os.path.exists(path)
             self.assertTrue(exists, msg='Missing file: %s' % filename)
             if exists:

i18n/tests/test_generate.py

-import os, string, random
+import os, string, random, re
+from polib import pofile
 from unittest import TestCase
 from datetime import datetime, timedelta
 
 import generate
-from execute import get_config, messages_dir, SOURCE_MSGS_DIR, SOURCE_LOCALE
+from config import CONFIGURATION
 
 class TestGenerate(TestCase):
     """
@@ -12,29 +13,16 @@ class TestGenerate(TestCase):
     generated_files = ('django-partial.po', 'djangojs.po', 'mako.po')
 
     def setUp(self):
-        self.configuration = get_config()
-
         # Subtract 1 second to help comparisons with file-modify time succeed,
         # since os.path.getmtime() is not millisecond-accurate
         self.start_time = datetime.now() - timedelta(seconds=1)
 
-    def test_configuration(self):
-        """
-        Make sure we have a valid configuration file,
-        and that it contains an 'en' locale.
-        """
-        self.assertIsNotNone(self.configuration)
-        locales = self.configuration['locales']
-        self.assertIsNotNone(locales)
-        self.assertIsInstance(locales, list)
-        self.assertIn('en', locales)
-
     def test_merge(self):
         """
         Tests merge script on English source files.
         """
-        filename = os.path.join(SOURCE_MSGS_DIR, random_name())
-        generate.merge(SOURCE_LOCALE, target=filename)
+        filename = os.path.join(CONFIGURATION.get_source_messages_dir(), random_name())
+        generate.merge(CONFIGURATION.get_source_locale(), target=filename)
         self.assertTrue(os.path.exists(filename))
         os.remove(filename)
 
@@ -47,13 +35,35 @@ class TestGenerate(TestCase):
         after start of test suite)
         """
         generate.main()
-        for locale in self.configuration['locales']:
-            for filename in ('django.mo', 'djangojs.mo'):
-                path = os.path.join(messages_dir(locale), filename)
+        for locale in CONFIGURATION.get_locales():
+            for filename in ('django', 'djangojs'):
+                mofile = filename+'.mo'
+                path = os.path.join(CONFIGURATION.get_messages_dir(locale), mofile)
                 exists = os.path.exists(path)
-                self.assertTrue(exists, msg='Missing file in locale %s: %s' % (locale, filename))
+                self.assertTrue(exists, msg='Missing file in locale %s: %s' % (locale, mofile))
                 self.assertTrue(datetime.fromtimestamp(os.path.getmtime(path)) >= self.start_time,
                                 msg='File not recently modified: %s' % path)
+            self.assert_merge_headers(locale)
+
+    def assert_merge_headers(self, locale):
+        """
+        This is invoked by test_main to ensure that it runs after
+        calling generate.main().
+        
+        There should be exactly three merge comment headers
+        in our merged .po file. This counts them to be sure.
+        A merge comment looks like this:
+        # #-#-#-#-#  django-partial.po (0.1a)  #-#-#-#-#
+
+        """
+        path = os.path.join(CONFIGURATION.get_messages_dir(locale), 'django.po')
+        po = pofile(path)
+        pattern = re.compile('^#-#-#-#-#', re.M)
+        match = pattern.findall(po.header)
+        self.assertEqual(len(match), 3,
+                         msg="Found %s (should be 3) merge comments in the header for %s" % \
+                         (len(match), path))
+
 
 def random_name(size=6):
     """Returns random filename as string, like test-4BZ81W"""

i18n/tests/test_validate.py

@@ -2,7 +2,8 @@ import os
 from unittest import TestCase
 from nose.plugins.skip import SkipTest
 
-from execute import call, LOCALE_DIR, LOG
+from config import LOCALE_DIR
+from execute import call, LOG
 
 class TestValidate(TestCase):
     """

i18n/transifex.py

+#!/usr/bin/python
+
+import sys
+from execute import execute
+
+def push():
+    execute('tx push -s')
+
+def pull():
+    execute('tx pull')
+
+
+if __name__ == '__main__':
+    if len(sys.argv)<2:
+        raise Exception("missing argument: push or pull")
+    arg = sys.argv[1]
+    if arg == 'push':
+        push()
+    elif arg == 'pull':
+        pull()
+    else:
+        raise Exception("unknown argument: (%s)" % arg)
+        

rakefile

@@ -551,14 +551,16 @@ namespace :i18n do
     desc "Push source strings to Transifex for translation"
     task :push do
       if validate_transifex_config()
-        sh("tx push -s")
+        cmd = File.join(REPO_ROOT, "i18n", "transifex.py")
+        sh("#{cmd} push")
       end
     end
 
     desc "Pull translated strings from Transifex"
     task :pull do
       if validate_transifex_config()
-        sh("tx pull")
+        cmd = File.join(REPO_ROOT, "i18n", "transifex.py")
+        sh("#{cmd} pull")
       end
     end
   end