Merge pull request easybuilders#1327 from Caylo/dump-comments

Preserve comments in dump() method

Author: boegel

easybuild/framework/easyconfig/easyconfig.py

@@ -46,7 +46,7 @@
 import easybuild.tools.environment as env
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import build_option, get_module_naming_scheme
-from easybuild.tools.filetools import decode_class_name, encode_class_name, read_file
+from easybuild.tools.filetools import decode_class_name, encode_class_name, read_file, write_file
 from easybuild.tools.module_naming_scheme import DEVEL_MODULE_SUFFIX
 from easybuild.tools.module_naming_scheme.utilities import avail_module_naming_schemes, det_full_ec_version
 from easybuild.tools.module_naming_scheme.utilities import det_hidden_modname, is_valid_module_name
@@ -78,6 +78,24 @@
 # values for these keys will not be templated in dump()
 EXCLUDED_KEYS_REPLACE_TEMPLATES = ['easyblock', 'name', 'version', 'description', 'homepage', 'toolchain']
 
+
+# ordered groups of keys to obtain a nice looking easyconfig file
+GROUPED_PARAMS = [
+    ['easyblock'],
+    ['name', 'version', 'versionprefix', 'versionsuffix'],
+    ['homepage', 'description'],
+    ['toolchain', 'toolchainopts'],
+    ['sources', 'source_urls'],
+    ['patches'],
+    ['builddependencies', 'dependencies', 'hiddendependencies'],
+    ['osdependencies'],
+    ['preconfigopts', 'configopts'],
+    ['prebuildopts', 'buildopts'],
+    ['preinstallopts', 'installopts'],
+    ['parallel', 'maxparallel'],
+]
+LAST_PARAMS = ['sanity_check_paths', 'moduleclass']
+
 _easyconfig_files_cache = {}
 _easyconfigs_cache = {}
 
@@ -246,6 +264,8 @@ def parse(self):
         local_vars = parser.get_config_dict()
         self.log.debug("Parsed easyconfig as a dictionary: %s" % local_vars)
 
+        self.comments = parser.get_comments()
+
         # make sure all mandatory parameters are defined
         # this includes both generic mandatory parameters and software-specific parameters defined via extra_options
         missing_mandatory_keys = [key for key in self.mandatory if key not in local_vars]
@@ -475,25 +495,6 @@ def dump(self, fp):
         """
         Dump this easyconfig to file, with the given filename.
         """
-        eb_file = file(fp, 'w')
-
-        # ordered groups of keys to obtain a nice looking easyconfig file
-        grouped_keys = [
-            ['easyblock'],
-            ['name', 'version', 'versionprefix', 'versionsuffix'],
-            ['homepage', 'description'],
-            ['toolchain', 'toolchainopts'],
-            ['sources', 'source_urls'],
-            ['patches'],
-            ['builddependencies', 'dependencies', 'hiddendependencies'],
-            ['osdependencies'],
-            ['preconfigopts', 'configopts'],
-            ['prebuildopts', 'buildopts'],
-            ['preinstallopts', 'installopts'],
-            ['parallel', 'maxparallel'],
-        ]
-
-        last_keys = ['sanity_check_paths', 'moduleclass']
 
         orig_enable_templating = self.enable_templating
         self.enable_templating = False # templated values should be dumped unresolved
@@ -509,6 +510,17 @@ def dump(self, fp):
         keys = sorted(self.template_values, key=lambda k: len(self.template_values[k]), reverse=True)
         templ_val = OrderedDict([(self.template_values[k], k) for k in keys if len(self.template_values[k]) > 2])
 
+        def add_key_and_comments(key, val):
+            """
+            Add key + value and comments (if any) to txt to be dumped.
+            """
+            if key in self.comments['inline']:
+                ebtxt.append("%s = %s  %s" % (key, val, self.comments['inline'][key]))
+            else:
+                if key in self.comments['above']:
+                    ebtxt.extend(self.comments['above'][key])
+                ebtxt.append("%s = %s" % (key, val))
+
         def include_defined_parameters(keyset):
             """
             Internal function to include parameters in the dumped easyconfig file which have a non-default value.
@@ -537,29 +549,33 @@ def include_defined_parameters(keyset):
                         else:
                             val = newval
 
-                        ebtxt.append("%s = %s" % (key, val))
+                        add_key_and_comments(key, val)
+
                         printed_keys.append(key)
                         printed = True
                 if printed:
                     ebtxt.append('')
 
-        # print easyconfig parameters ordered and in groups specified above
         ebtxt = []
         printed_keys = []
-        include_defined_parameters(grouped_keys)
+
+        # add header comments
+        ebtxt.extend(self.comments['header'])
+
+        # print easyconfig parameters ordered and in groups specified above
+        include_defined_parameters(GROUPED_PARAMS)
 
         # print other easyconfig parameters at the end
-        keys_to_ignore = printed_keys + last_keys
+        keys_to_ignore = printed_keys + LAST_PARAMS
         for key in default_values:
             if key not in keys_to_ignore and self[key] != default_values[key]:
-                ebtxt.append("%s = %s" % (key, quote_py_str(self[key])))
+                add_key_and_comments(key, quote_py_str(self[key]))
         ebtxt.append('')
 
-        # print last two parameters
-        include_defined_parameters([[k] for k in last_keys])
+        # print last parameters
+        include_defined_parameters([[k] for k in LAST_PARAMS])
 
-        eb_file.write(('\n'.join(ebtxt)).strip()) # strip for newlines at the end
-        eb_file.close()
+        write_file(fp, ('\n'.join(ebtxt)).strip()) # strip for newlines at the end
 
         self.enable_templating = orig_enable_templating
 

easybuild/framework/easyconfig/parser.py

@@ -29,6 +29,7 @@
 
 @author: Stijn De Weirdt (Ghent University)
 """
+import copy
 import os
 import re
 from vsc.utils import fancylogger
@@ -84,6 +85,9 @@ def __init__(self, filename=None, format_version=None, rawcontent=None):
 
         self.rawcontent = None  # the actual unparsed content
 
+        # comments in the easyconfig file
+        self.comments = None
+
         self.get_fn = None  # read method and args
         self.set_fn = None  # write method and args
 
@@ -99,6 +103,8 @@ def __init__(self, filename=None, format_version=None, rawcontent=None):
         else:
             raise EasyBuildError("Neither filename nor rawcontent provided to EasyConfigParser")
 
+        self._extract_comments()
+
     def process(self, filename=None):
         """Create an instance"""
         self._read(filename=filename)
@@ -131,6 +137,52 @@ def _read(self, filename=None):
             msg = 'rawcontent is not basestring: type %s, content %s' % (type(self.rawcontent), self.rawcontent)
             raise EasyBuildError("Unexpected result for raw content: %s", msg)
 
+    def _extract_comments(self):
+        """Extract comments from raw content."""
+        # Keep track of comments and their location (top of easyconfig, key they are intended for, line they are on
+        # discriminate between header comments (top of easyconfig file), single-line comments (at end of line) and other
+        # At the moment there is no support for inline comments on lines that don't contain the key value
+
+        self.comments = {
+            'above' : {},
+            'header' : [],
+            'inline' : {},
+         }
+
+        rawlines = self.rawcontent.split('\n')
+
+        # extract header first
+        while rawlines[0].startswith('#'):
+            self.comments['header'].append(rawlines.pop(0))
+
+        parsed_ec = None
+        while rawlines:
+            rawline = rawlines.pop(0)
+            if rawline.startswith('#'):
+                comment = []
+                # comment could be multi-line
+                while rawline.startswith('#') or not rawline:
+                    # drop empty lines (that don't even include a #)
+                    if rawline:
+                        comment.append(rawline)
+                    rawline = rawlines.pop(0)
+                key = rawline.split('=', 1)[0].strip()
+                self.comments['above'][key] = comment
+
+            elif '#' in rawline:  # inline comment
+                if parsed_ec is None:
+                    # obtain parsed easyconfig as a dict, if it wasn't already
+                    # note: this currently trigger a reparse
+                    parsed_ec = self.get_config_dict()
+
+                key = rawline.split('=', 1)[0].strip()
+                comment = rawline.rsplit('#', 1)[1].strip()
+
+                # check if hash actually indicated a comment, or if it is part of the value
+                if key in parsed_ec:
+                    if comment.replace("'", "").replace('"', '') not in str(parsed_ec[key]):
+                        self.comments['inline'][key] = '# ' + comment
+
     def _det_format_version(self):
         """Extract the format version from the raw content"""
         if self.format_version is None:
@@ -184,3 +236,7 @@ def get_config_dict(self, validate=True):
         if validate:
             self._formatter.validate()
         return self._formatter.get_config_dict()
+
+    def get_comments(self):
+        """Return comments, and their location info"""
+        return copy.deepcopy(self.comments)

test/framework/easyconfig.py

@@ -1273,6 +1273,53 @@ def test_dump_template(self):
         # reparsing the dumped easyconfig file should work
         ecbis = EasyConfig(testec)
 
+    def test_dump_comments(self):
+        """ Test dump() method for files containing comments """
+        rawtxt = '\n'.join([
+            "# #",
+            "# some header comment",
+            "# #",
+            "easyblock = 'EB_foo'",
+            '',
+            "name = 'Foo'  # name comment",
+            "version = '0.0.1'",
+            "versionsuffix = '-test'",
+            '',
+            "# comment on the homepage",
+            "homepage = 'http://foo.com/'",
+            'description = "foo description with a # in it"  # test',
+            '',
+            "# toolchain comment",
+            '',
+            "toolchain = {'version': 'dummy', 'name': 'dummy'}",
+            '',
+            "sanity_check_paths = {'files': ['files/foo/foobar'], 'dirs':[] }",
+            '',
+            "foo_extra1 = 'foobar'",
+        ])
+
+        handle, testec = tempfile.mkstemp(prefix=self.test_prefix, suffix='.eb')
+        os.close(handle)
+
+        ec = EasyConfig(None, rawtxt=rawtxt)
+        ec.dump(testec)
+        ectxt = read_file(testec)
+
+        patterns = [
+            r"# #\n# some header comment\n# #",
+            r"name = 'Foo'  # name comment",
+            r"# comment on the homepage\nhomepage = 'http://foo.com/'",
+            r'description = "foo description with a # in it"  # test',
+            r"# toolchain comment\ntoolchain = {'version': 'dummy', 'name': 'dummy'}"
+        ]
+
+        for pattern in patterns:
+            regex = re.compile(pattern, re.M)
+            self.assertTrue(regex.search(ectxt), "Pattern '%s' found in: %s" % (regex.pattern, ectxt))
+
+        # reparsing the dumped easyconfig file should work
+        ecbis = EasyConfig(testec)
+
     def test_to_template_str(self):
         """ Test for to_template_str method """