[damned-lies] Refactor reading variables from files

[Claude Paroz <claudep src gnome org>]

commit 8e40c509abac1c3cd67b6d1e3963745ca2a16991
Author: Claude Paroz <claude 2xlibre net>
Date:   Tue Jun 27 21:03:34 2017 +0200

    Refactor reading variables from files

 stats/models.py      |    8 ++-
 stats/tests/tests.py |    7 ++-
 stats/utils.py       |  133 ++++++++++++++++++++++++++++---------------------
 3 files changed, 86 insertions(+), 62 deletions(-)
---
diff --git a/stats/models.py b/stats/models.py
index a3dd8ea..bcd5b1d 100644
--- a/stats/models.py
+++ b/stats/models.py
@@ -828,7 +828,10 @@ class Domain(models.Model):
                                                 for path in self.extra_its_dirs.split(':')]
                 )
             # Parse and use content from: "XGETTEXT_OPTIONS = --keyword=_ --keyword=N_"
-            kwds_vars = utils.read_makefile_variable([vcs_path], 'XGETTEXT_OPTIONS', 
makefile_name='Makevars')
+            kwds_vars = None
+            makefile = utils.MakefileWrapper.find_file([vcs_path], file_name='Makevars')
+            if makefile:
+                kwds_vars = makefile.read_variable('XGETTEXT_OPTIONS')
             if kwds_vars:
                 pot_command.extend(kwds_vars.split())
             # Added last as some chars in it may disturb CLI parsing
@@ -891,7 +894,8 @@ class Domain(models.Model):
                     file_path, variable = self.linguas_location.split("#")
                 else:
                     file_path = self.linguas_location
-                langs = utils.search_variable_in_file(os.path.join(base_path, file_path), variable)
+                linguas_file = utils.MakefileWrapper(os.path.join(base_path, file_path))
+                langs = linguas_file.read_variable(variable)
                 return {'langs': langs,
                         'error': ugettext_noop("Entry for this language is not present in %(var)s variable 
in %(file)s file." % {
                             'var': variable, 'file': file_path})}
diff --git a/stats/tests/tests.py b/stats/tests/tests.py
index 3c395f2..4a8dfcd 100644
--- a/stats/tests/tests.py
+++ b/stats/tests/tests.py
@@ -536,11 +536,12 @@ class FigureTests(TestCase):
 
 class UtilsTests(TestCase):
     def test_read_file_variable(self):
-        file_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "help_docbook", "Makefile.am")
-        var_content = utils.search_variable_in_file(file_path, "DOC_INCLUDES")
+        base_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "help_docbook")
+        makefile = utils.MakefileWrapper.find_file([base_path])
+        var_content = makefile.read_variable("DOC_INCLUDES")
         self.assertEqual(var_content.split(), ['rnusers.xml', 'rnlookingforward.xml', '$(NULL)'])
 
-        var_content = utils.search_variable_in_file(file_path, "HELP_FILES")
+        var_content = makefile.read_variable("HELP_FILES")
         self.assertEqual(var_content.split(), ['rnusers.xml', 'rnlookingforward.xml'])
 
     def test_generate_doc_potfile_docbook(self):
diff --git a/stats/utils.py b/stats/utils.py
index babecf6..7fcb453 100644
--- a/stats/utils.py
+++ b/stats/utils.py
@@ -19,6 +19,7 @@ from django.contrib.sites.models import Site
 from django.core.files.base import File
 from django.template.loader import get_template
 from django.utils.encoding import force_text
+from django.utils.functional import cached_property
 from django.utils.translation import ugettext_noop
 
 from common.utils import send_mail
@@ -34,7 +35,7 @@ CHANGED_NO_ADDITIONS    = 3
 ITSTOOL_PATH = getattr(settings, 'ITSTOOL_PATH', '')
 ITS_DATA_DIR = os.path.join(settings.SCRATCHDIR, 'data', 'its')
 
-class DocFormat(object):
+class DocFormat:
     itstool_regex = re.compile("^msgid \"external ref=\'(?P<path>[^\']*)\' md5=\'(?P<hash>[^\']*)\'\"")
     xml2po_regex = re.compile("^msgid \"@@image: \'(?P<path>[^\']*)\'; md5=(?P<hash>[^\"]*)\"")
     def __init__(self, is_itstool, is_mallard):
@@ -86,6 +87,64 @@ class DocFormat(object):
         return self.tool == "itstool" and self.itstool_regex or self.xml2po_regex
 
 
+class MakefileWrapper:
+    default_makefiles = ['Makefile.am']
+
+    @classmethod
+    def find_file(cls, vcs_paths, file_name=None):
+        """Return the first makefile found as a MakefileWrapper instance, or None."""
+        names = [file_name] if file_name is not None else cls.default_makefiles
+        for path in vcs_paths:
+            for file_name in names:
+                file_path = os.path.join(path, file_name)
+                if os.access(file_path, os.R_OK):
+                    return MakefileWrapper(file_path)
+
+    def __init__(self, path):
+        self.path = path
+
+    @cached_property
+    def content(self):
+        try:
+            with open(self.path, "r") as fh:
+                content = fh.read()
+        except IOError:
+            return None
+        return content
+
+    def read_variable(self, variable):
+        if self.content is None:
+            return None
+        non_terminated_content = ""
+        found = None
+        for line in self.content.splitlines():
+            line = line.strip()
+            if non_terminated_content:
+                line = non_terminated_content + " " + line
+            # Test if line is non terminated (end with \ or even quote count)
+            if (len(line) > 2 and line[-1] == "\\") or line.count('"') % 2 == 1:
+                if line[-1] == "\\":
+                    # Remove trailing backslash
+                    line = line[:-1]
+                non_terminated_content = line
+            else:
+                non_terminated_content = ""
+                # Search for variable
+                match = re.search('%s\s*[=,]\s*"?([^"]*)"?' % variable, line)
+                if match:
+                    found = match.group(1)
+                    break
+
+        # Try to expand '$(var)' variables
+        if found:
+            for element in found.split():
+                if element.startswith('$(') and element.endswith(')') and element != '$(NULL)':
+                    result = self.read_variable(element[2:-1])
+                    if result:
+                        found = found.replace(element, result)
+        return found
+
+
 def sort_object_list(lst, sort_meth):
     """ Sort an object list with sort_meth (which should return a translated string) """
     templist = [(getattr(obj_, sort_meth)().lower(), obj_) for obj_ in lst]
@@ -194,15 +253,22 @@ def generate_doc_pot_file(vcs_path, potbase, moduleid):
     """ Return the pot file for a document-type domain, and the error if any """
     errors = []
 
-    doc_id = read_makefile_variable([vcs_path], "HELP_ID", default="NOTFOUND")
-    uses_itstool = doc_id == "NOTFOUND" or bool(doc_id)
+    makefile = MakefileWrapper.find_file([vcs_path])
+    if makefile is None:
+        errors.append(
+            ("error", ugettext_noop("Unable to find a makefile for module %s") % moduleid)
+        )
+        return "", errors, None
+
+    doc_id = makefile.read_variable("HELP_ID")
+    uses_itstool = doc_id is not None
     all_C_files = os.listdir(os.path.join(vcs_path, "C"))
     has_page_files = any(f.endswith('.page') for f in all_C_files)
     doc_format = DocFormat(is_itstool=uses_itstool, is_mallard=has_page_files)
 
     files = []
     if doc_format.format == "docbook":
-        modulename = read_makefile_variable([vcs_path], doc_format.module_var)
+        modulename = makefile.read_variable(doc_format.module_var)
         if not modulename:
             return "", (("error", ugettext_noop("Module %s doesn’t look like gnome-doc-utils module.") % 
moduleid),), doc_format
         for index_page in ("index.docbook", modulename + ".xml", moduleid + ".xml"):
@@ -218,7 +284,7 @@ def generate_doc_pot_file(vcs_path, potbase, moduleid):
                 errors.append(("error", ugettext_noop("%s doesn’t point to a real file, probably a macro.") 
% doc_format.module_var))
                 return "", errors, doc_format
 
-    includes = read_makefile_variable([vcs_path], doc_format.include_var)
+    includes = makefile.read_variable(doc_format.include_var)
     if includes:
         files.extend(filter(lambda x:x not in ("", "$(NULL)"), includes.split()))
     elif includes is None and doc_format.format == "mallard":
@@ -244,56 +310,6 @@ def generate_doc_pot_file(vcs_path, potbase, moduleid):
         return potfile, errors, doc_format
 
 
-def read_makefile_variable(vcs_paths, variable, makefile_name='Makefile.am', default=None):
-    """ vcs_paths is a list of potential path where Makefile.am could be found """
-    makefiles = [os.path.join(path, makefile_name) for path in vcs_paths]
-    for makefile in makefiles:
-        if os.access(makefile, os.R_OK):
-            break
-    else:
-        return default  # no file found
-
-    return search_variable_in_file(makefile, variable)
-
-
-def search_variable_in_file(file_path, variable):
-    """ Search for a variable value in a file, and return content if found
-        Return None if variable not found in file (or file does not exist)
-    """
-    non_terminated_content = ""
-    found = None
-    try:
-        with open(file_path, "r") as fh:
-            for line in fh:
-                line = line.strip()
-                if non_terminated_content:
-                    line = non_terminated_content + " " + line
-                # Test if line is non terminated (end with \ or even quote count)
-                if (len(line) > 2 and line[-1] == "\\") or line.count('"') % 2 == 1:
-                    if line[-1] == "\\":
-                        # Remove trailing backslash
-                        line = line[:-1]
-                    non_terminated_content = line
-                else:
-                    non_terminated_content = ""
-                    # Search for variable
-                    match = re.search('%s\s*[=,]\s*"?([^"]*)"?' % variable, line)
-                    if match:
-                        found = match.group(1)
-                        break
-    except IOError:
-        return None
-
-    # Try to expand '$(var)' variables
-    if found:
-        for element in found.split():
-            if element.startswith('$(') and element.endswith(')') and element != '$(NULL)':
-                result = search_variable_in_file(file_path, element[2:-1])
-                if result:
-                    found = found.replace(element, result)
-    return found
-
-
 def pot_diff_status(pota, potb):
     (status, output, errs) = run_shell_command('diff "%s" "%s"|wc -l' % (pota, potb))
     # POT generation date always change and produce a 4 line diff
@@ -431,7 +447,7 @@ def get_ui_linguas(module_path, po_path):
                 'error': ugettext_noop("No need to edit LINGUAS file or variable for this module")}
 
     for configure in [configureac, configurein]:
-        found = search_variable_in_file(configure, 'ALL_LINGUAS')
+        found = MakefileWrapper(configure).read_variable('ALL_LINGUAS')
         if found is not None:
             return {'langs': found.split(),
                     'error': ugettext_noop("Entry for this language is not present in ALL_LINGUAS in 
configure file.") }
@@ -440,7 +456,10 @@ def get_ui_linguas(module_path, po_path):
 
 def get_doc_linguas(module_path, po_path):
     """Get language list in one Makefile.am (either path) """
-    linguas = read_makefile_variable([po_path, module_path], "(?:DOC|HELP)_LINGUAS")
+    linguas = None
+    linguas_file = MakefileWrapper.find_file([po_path, module_path])
+    if linguas_file:
+        linguas = linguas_file.read_variable("(?:DOC|HELP)_LINGUAS")
     if linguas is None:
         return {'langs':None,
                 'error': ugettext_noop("Don’t know where to look for the DOC_LINGUAS variable, ask the 
module maintainer.") }